This is an archive of the discontinued LLVM Phabricator instance.

[CodeGenPrepare] Don't create inttoptr for ni ptrs
ClosedPublic

Authored by loladiro on May 11 2017, 4:10 PM.

Details

Summary

Arguably non-integral pointers probably shouldn't show up here at all,
but since the backend doesn't complain and this takes valid (according
to the Verifier) IR and makes it invalid, make sure not to introduce
any inttoptr instructions if we're dealing with non-integral pointers.

Event Timeline

loladiro created this revision.May 11 2017, 4:10 PM

Bump. @sanjoy does this look good to you?

Bump on review for this.

I'm seeing a similar failure caused by the same code even with this patch applied on LLVM 4.0.

The relevant IR is

%32 = call %jl_value_t addrspace(10)* %31(i64 %24), !dbg !22697
%34 = addrspacecast %jl_value_t addrspace(10)* %32 to %jl_value_t*
%35 = ptrtoint %jl_value_t* %34 to i64, !dbg !22699
%36 = add i64 %35, 8, !dbg !22699
%76 = inttoptr i64 %36 to i8*
%77 = getelementptr i8, i8* %76, i64 2, !dbg !22717

And when it enters the else branch around L4368 Addr is %77 = getelementptr i8, i8* %76, i64 2, !dbg !22717 and AddrMode.BaseReg is %32 = call %jl_value_t addrspace(10)* %31(i64 %24), !dbg !22697. This is not handled by the non-integral pointer check added in the PR and causes a ptrtoint on %32. The following patch seems to fix it

diff
diff --git a/lib/CodeGen/CodeGenPrepare.cpp b/lib/CodeGen/CodeGenPrepare.cpp
index 44d6b3e264c..4177819e989 100644
--- a/lib/CodeGen/CodeGenPrepare.cpp
+++ b/lib/CodeGen/CodeGenPrepare.cpp
@@ -4067,6 +4067,23 @@ bool CodeGenPrepare::optimizeMemoryInst(Instruction *MemoryInst, Value *Addr,
     // non-integral pointers, so in that case bail out now.
     if (DL->isNonIntegralPointerType(Addr->getType()))
       return false;
+    if (AddrMode.BaseReg) {
+      Type *BaseTy = AddrMode.BaseReg->getType();
+      if (BaseTy->isPointerTy() && DL->isNonIntegralPointerType(BaseTy)) {
+        return false;
+      }
+    }
+    if (AddrMode.Scale) {
+      Type *ScaleTy = AddrMode.ScaledReg->getType();
+      if (ScaleTy->isPointerTy() && DL->isNonIntegralPointerType(ScaleTy)) {
+        return false;
+      }
+    }
+    if (AddrMode.BaseGV) {
+      if (DL->isNonIntegralPointerType(AddrMode.BaseGV->getType())) {
+        return false;
+      }
+    }
 
     DEBUG(dbgs() << "CGP: SINKING nonlocal addrmode: " << AddrMode << " for "
                  << *MemoryInst << "\n");

Not sure if it is the right fix.

sanjoy accepted this revision.Jun 24 2017, 9:41 PM

lgtm

This revision is now accepted and ready to land.Jun 24 2017, 9:41 PM
This revision was automatically updated to reflect the committed changes.