From ed5d3150e37efa91136ded122a450a411c0ebe3a Mon Sep 17 00:00:00 2001
From: Bert Maher <bertrand@fb.com>
Date: Tue, 14 May 2013 16:50:02 -0700
Subject: [PATCH] Revert "Try inlining the LdGblAddr(Def) helpers" (commit
 bc1e471).

This used to look perf-neutral with a slight instruction
win... after I actually landed it, it looks like a big, consistent
negative for perf.
---
 hphp/runtime/vm/translator/hopt/codegen.cpp   | 41 +++++++------------
 .../vm/translator/hopt/hhbctranslator.cpp     |  5 +--
 hphp/runtime/vm/translator/translator-x64.cpp |  2 +-
 hphp/runtime/vm/translator/translator-x64.h   |  2 -
 4 files changed, 16 insertions(+), 34 deletions(-)

diff --git a/hphp/runtime/vm/translator/hopt/codegen.cpp b/hphp/runtime/vm/translator/hopt/codegen.cpp
index b8db917fe..43ee17dba 100644
--- a/hphp/runtime/vm/translator/hopt/codegen.cpp
+++ b/hphp/runtime/vm/translator/hopt/codegen.cpp
@@ -4646,40 +4646,27 @@ void CodeGenerator::cgAKExists(IRInstruction* inst) {
                ArgGroup(m_regs).ssa(key).ssa(arr));
 }
 
+HOT_FUNC_VM static TypedValue* ldGblAddrHelper(StringData* name) {
+  return g_vmContext->m_globalVarEnv->lookup(name);
+}
+
+HOT_FUNC_VM static TypedValue* ldGblAddrDefHelper(StringData* name) {
+  TypedValue* r = g_vmContext->m_globalVarEnv->lookupAdd(name);
+  decRefStr(name);
+  return r;
+}
+
 void CodeGenerator::cgLdGblAddr(IRInstruction* inst) {
   auto dstReg = m_regs[inst->getDst()].getReg();
-  auto rEC = rScratch;
-  // TODO(#2384005): If we create separate opcodes for loading the VM
-  // context and the global variable environment we may find some CSE
-  // opportunities that aren't currently exploited.
-  emitGetGContext(m_as, rEC);
-  m_as.loadq(rEC[offsetof(VMExecutionContext, m_globalVarEnv)], rEC);
-  ArgGroup args = ArgGroup(m_regs)
-    .reg(rEC)
-    .ssa(inst->getSrc(0));
-  cgCallHelper(m_as,
-               (TCA)getMethodPtr(&VarEnv::lookup),
-               dstReg,
-               kNoSyncPoint,
-               args);
+  cgCallHelper(m_as, (TCA)ldGblAddrHelper, dstReg, kNoSyncPoint,
+               ArgGroup(m_regs).ssa(inst->getSrc(0)));
   m_as.testq(dstReg, dstReg);
   emitFwdJcc(CC_Z, inst->getTaken());
 }
 
 void CodeGenerator::cgLdGblAddrDef(IRInstruction* inst) {
-  auto dstReg = m_regs[inst->getDst()].getReg();
-  auto rEC = rScratch;
-  // TODO(#2384005): see LdGblAddr
-  emitGetGContext(m_as, rEC);
-  m_as.loadq(rEC[offsetof(VMExecutionContext, m_globalVarEnv)], rEC);
-  ArgGroup args = ArgGroup(m_regs)
-    .reg(rEC)
-    .ssa(inst->getSrc(0));
-  cgCallHelper(m_as,
-               (TCA)getMethodPtr(&VarEnv::lookupAdd),
-               dstReg,
-               kNoSyncPoint,
-               args);
+  cgCallHelper(m_as, (TCA)ldGblAddrDefHelper, inst->getDst(), kNoSyncPoint,
+               ArgGroup(m_regs).ssa(inst->getSrc(0)));
 }
 
 void CodeGenerator::cgJmpZeroHelper(IRInstruction* inst,
diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp
index 4d47ca983..345a9a098 100644
--- a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp
+++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp
@@ -1199,10 +1199,7 @@ SSATmp* HhbcTranslator::emitLdGblAddr(const StringData* gblName, Block* block) {
 }
 
 SSATmp* HhbcTranslator::emitLdGblAddrDef(const StringData* gblName) {
-  SSATmp* name = getStrName(gblName);
-  SSATmp* addr = gen(LdGblAddrDef, name);
-  gen(DecRef, name);
-  return addr;
+  return gen(LdGblAddrDef, getStrName(gblName));
 }
 
 void HhbcTranslator::emitIncDecS(bool pre, bool inc) {
diff --git a/hphp/runtime/vm/translator/translator-x64.cpp b/hphp/runtime/vm/translator/translator-x64.cpp
index a6d2ec943..5e6bc32bf 100644
--- a/hphp/runtime/vm/translator/translator-x64.cpp
+++ b/hphp/runtime/vm/translator/translator-x64.cpp
@@ -890,7 +890,7 @@ void TranslatorX64::emitIncRefGeneric(PhysReg base, int disp) {
   emitIncRefGenericRegSafe(base, disp, r(tmpReg));
 }
 
-void emitGetGContext(X64Assembler& a, PhysReg dest) {
+static void emitGetGContext(X64Assembler& a, PhysReg dest) {
   emitTLSLoad<ExecutionContext>(a, g_context, dest);
 }
 
diff --git a/hphp/runtime/vm/translator/translator-x64.h b/hphp/runtime/vm/translator/translator-x64.h
index c74b0907f..3641864ce 100644
--- a/hphp/runtime/vm/translator/translator-x64.h
+++ b/hphp/runtime/vm/translator/translator-x64.h
@@ -1202,8 +1202,6 @@ bool classIsUnique(const Class* cls);
 bool classIsUniqueOrCtxParent(const Class* cls);
 bool classIsUniqueNormalClass(const Class* cls);
 
-void emitGetGContext(X64Assembler& a, PhysReg dest);
-
 // SpaceRecorder is used in translator-x64.cpp and in hopt/irtranslator.cpp
 // RAII logger for TC space consumption.
 struct SpaceRecorder {