Fix lowering and optimization of 64-bit absolute addresses

x86-64 does not support 64-bit immediates as absolute memory addresses.
They have to be stored in a register, which can then be used as [base].

Previously we addressed this at the SubzeroReactor level by emitting a
Bitcast from an Ice::Operand to an Ice::Variable, for which Subzero
already supported 64-bit constants as input.

This change implements X86OperandMem creation from a 64-bit constant
operand by letting legalize() move it into a GPR and using it as the
memory operand's base register.

A Reactor unit test is added to exercise this.

Another issue was that doLoadOpt() assumed all load instructions are
candidates for fusing into a subsequent instruction which takes the
result of the load. This isn't true when for 64-bit constant addresses
an instruction to copy it into a register is inserted.

For now this case is simply skipped. A future optimization could adjust
the iterators properly so the load from [base] can be fused with the
next instruction.

Lastly, it is possible for a 64-bit constant to fit within a 32-bit
immediate, in which case legalize() by default does not perform the copy
into a GPR (note this is to allow moves and calls with 64-bit
immediates, where they are legal), and simply returns the 64-bit
constant. So we must not allow legalization to an immediate in this
case. Note that while we could replace it with a 32-bit constant, it's
rare for absolute addresses to fit in this range, and it would be
non-deterministic which path is taken, so for consistency we don't
perform this optimization.

Bug: b/148272103
Change-Id: I5fcfa971dc93f2307202ee11619e84c65fe46188
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/52768
Tested-by: Nicolas Capens <nicolascapens@google.com>
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/Reactor/Optimizer.cpp b/src/Reactor/Optimizer.cpp
index 8be37b5..d5f95e8 100644
--- a/src/Reactor/Optimizer.cpp
+++ b/src/Reactor/Optimizer.cpp
@@ -319,14 +319,6 @@
 					continue;
 				}
 
-				// TODO(b/148272103): InstLoad assumes that a constant ptr is an offset, rather than an
-				// absolute address. We need to make sure we don't replace a variable with a constant
-				// on this load.
-				if(llvm::isa<Ice::Constant>(storeValue))
-				{
-					continue;
-				}
-
 				replace(load, storeValue);
 
 				for(size_t i = 0; i < addressUses.loads.size(); i++)
@@ -490,15 +482,6 @@
 						continue;
 					}
 
-					// TODO(b/148272103): InstLoad assumes that a constant ptr is an offset, rather than an
-					// absolute address. We need to make sure we don't replace a variable with a constant
-					// on this load.
-					if(llvm::isa<Ice::Constant>(storeValue))
-					{
-						unmatchedLoads = true;
-						continue;
-					}
-
 					replace(inst, storeValue);
 				}
 			}
diff --git a/src/Reactor/SubzeroReactor.cpp b/src/Reactor/SubzeroReactor.cpp
index 41c07c8..53377ca 100644
--- a/src/Reactor/SubzeroReactor.cpp
+++ b/src/Reactor/SubzeroReactor.cpp
@@ -174,15 +174,6 @@
 	return Call(function, basicBlock, retTy, reinterpret_cast<void const *>(fptr), iceArgs, false);
 }
 
-// Returns a non-const variable copy of const v
-Ice::Variable *createUnconstCast(Ice::Cfg *function, Ice::CfgNode *basicBlock, Ice::Constant *v)
-{
-	Ice::Variable *result = function->makeVariable(v->getType());
-	Ice::InstCast *cast = Ice::InstCast::create(function, Ice::InstCast::Bitcast, result, v);
-	basicBlock->appendInst(cast);
-	return result;
-}
-
 Ice::Variable *createTruncate(Ice::Cfg *function, Ice::CfgNode *basicBlock, Ice::Operand *from, Ice::Type toType)
 {
 	Ice::Variable *to = function->makeVariable(toType);
@@ -193,14 +184,6 @@
 
 Ice::Variable *createLoad(Ice::Cfg *function, Ice::CfgNode *basicBlock, Ice::Operand *ptr, Ice::Type type, unsigned int align)
 {
-	// TODO(b/148272103): InstLoad assumes that a constant ptr is an offset, rather than an
-	// absolute address. We circumvent this by casting to a non-const variable, and loading
-	// from that.
-	if(auto *cptr = llvm::dyn_cast<Ice::Constant>(ptr))
-	{
-		ptr = sz::createUnconstCast(function, basicBlock, cptr);
-	}
-
 	Ice::Variable *result = function->makeVariable(type);
 	auto load = Ice::InstLoad::create(function, result, ptr, align);
 	basicBlock->appendInst(load);
diff --git a/tests/ReactorUnitTests/ReactorUnitTests.cpp b/tests/ReactorUnitTests/ReactorUnitTests.cpp
index b6c0d9cf..6b85a48 100644
--- a/tests/ReactorUnitTests/ReactorUnitTests.cpp
+++ b/tests/ReactorUnitTests/ReactorUnitTests.cpp
@@ -376,6 +376,23 @@
 	EXPECT_EQ(result, 7);
 }
 
+TEST(ReactorUnitTests, ConstantPointer)
+{
+	int c = 44;
+
+	FunctionT<int()> function;
+	{
+		Int x = *Pointer<Int>(ConstantPointer(&c));
+
+		Return(x);
+	}
+
+	auto routine = function(testName().c_str());
+
+	int result = routine();
+	EXPECT_EQ(result, 44);
+}
+
 // This test excercises the Optimizer::eliminateLoadsFollowingSingleStore() optimization pass.
 // The three load operations for `y` should get eliminated.
 // TODO(b/180665600): Check that the optimization took place.
diff --git a/third_party/subzero/src/IceTargetLoweringX86BaseImpl.h b/third_party/subzero/src/IceTargetLoweringX86BaseImpl.h
index d4c41d3..37ab794 100644
--- a/third_party/subzero/src/IceTargetLoweringX86BaseImpl.h
+++ b/third_party/subzero/src/IceTargetLoweringX86BaseImpl.h
@@ -823,11 +823,15 @@
       // Determine whether the current instruction is a Load instruction or
       // equivalent.
       if (auto *Load = llvm::dyn_cast<InstLoad>(CurInst)) {
-        // An InstLoad always qualifies.
-        LoadDest = Load->getDest();
-        constexpr bool DoLegalize = false;
-        LoadSrc = formMemoryOperand(Load->getLoadAddress(), LoadDest->getType(),
-                                    DoLegalize);
+        // An InstLoad qualifies unless it uses a 64-bit absolute address,
+        // which requires legalization to insert a copy to register.
+        // TODO(b/148272103): Fold these after legalization.
+        if (!Traits::Is64Bit || !llvm::isa<Constant>(Load->getLoadAddress())) {
+          LoadDest = Load->getDest();
+          constexpr bool DoLegalize = false;
+          LoadSrc = formMemoryOperand(Load->getLoadAddress(),
+                                      LoadDest->getType(), DoLegalize);
+        }
       } else if (auto *Intrin = llvm::dyn_cast<InstIntrinsic>(CurInst)) {
         // An AtomicLoad intrinsic qualifies as long as it has a valid memory
         // ordering, and can be implemented in a single instruction (i.e., not
@@ -8121,20 +8125,22 @@
     auto *Offset = llvm::dyn_cast<Constant>(Opnd);
     assert(Base || Offset);
     if (Offset) {
-      // During memory operand building, we do not blind or pool the constant
-      // offset, we will work on the whole memory operand later as one entity
-      // later, this save one instruction. By turning blinding and pooling off,
-      // we guarantee legalize(Offset) will return a Constant*.
       if (!llvm::isa<ConstantRelocatable>(Offset)) {
-        Offset = llvm::cast<Constant>(legalize(Offset));
-      }
+        if (llvm::isa<ConstantInteger64>(Offset)) {
+          // Memory operands cannot have 64-bit immediates, so they must be
+          // legalized into a register only.
+          Base = llvm::cast<Variable>(legalize(Offset, Legal_Reg));
+          Offset = nullptr;
+        } else {
+          Offset = llvm::cast<Constant>(legalize(Offset));
 
-      assert(llvm::isa<ConstantInteger32>(Offset) ||
-             llvm::isa<ConstantRelocatable>(Offset));
+          assert(llvm::isa<ConstantInteger32>(Offset) ||
+                 llvm::isa<ConstantRelocatable>(Offset));
+        }
+      }
     }
     Mem = X86OperandMem::create(Func, Ty, Base, Offset);
   }
-  // Do legalization, which contains pooling or do pooling.
   return llvm::cast<X86OperandMem>(DoLegalize ? legalize(Mem) : Mem);
 }