[PATCH] D40447: [CGP] Fix handling of null pointer values in optimizeMemoryInst

[John Brawn via Phabricator via llvm-commits]

john.brawn created this revision.
Herald added a subscriber: javed.absar.

The current way that trivial addressing modes are detected incorrectly thinks that null pointers are non-trivial, leading to an infinite loop where we keep duplicating the same select. Fix this by being aware of null when deciding if an addressing mode is trivial.


Repository:
  rL LLVM

https://reviews.llvm.org/D40447

Files:
  lib/CodeGen/CodeGenPrepare.cpp
  test/Transforms/CodeGenPrepare/ARM/sink-addrmode.ll


Index: test/Transforms/CodeGenPrepare/ARM/sink-addrmode.ll
===================================================================
--- test/Transforms/CodeGenPrepare/ARM/sink-addrmode.ll
+++ test/Transforms/CodeGenPrepare/ARM/sink-addrmode.ll
@@ -366,3 +366,55 @@
   store i32 %value, i32* %select, align 4
   ret void
 }
+
+; Same for a select between a value and global variable
+; CHECK-LABEL: @test_select_trivial_ptr_gv
+; CHECK: select i1 %cmp, i32* %ptr, i32* @gv2
+define void @test_select_trivial_ptr_gv(i32* %ptr, i32 %value) {
+entry:
+  %cmp = icmp sgt i32 %value, 0
+  %select = select i1 %cmp, i32* %ptr, i32* @gv2
+  store i32 %value, i32* %select, align 4
+  ret void
+}
+
+; Same for a select between a global variable and null, though the test needs to
+; be a little more complicated to avoid dereferencing a potential null pointer
+; CHECK-LABEL: @test_select_trivial_gv_null
+; CHECK: select i1 %cmp.i, i32* @gv1, i32* null
+define void @test_select_trivial_gv_null(){
+entry:
+  %gv1_val = load i32, i32* @gv1, align 4
+  %cmp.i = icmp eq i32 %gv1_val, 0
+  %spec.select.i = select i1 %cmp.i, i32* @gv1, i32* null
+  br i1 %cmp.i, label %if.then, label %if.end
+
+if.then:
+  %val = load i32, i32* %spec.select.i, align 4
+  %inc = add nsw i32 %val, 1
+  store i32 %inc, i32* %spec.select.i, align 4
+  br label %if.end
+
+if.end:
+  ret void
+}
+
+; Same for a select between a value and null
+; CHECK-LABEL: @test_select_trivial_ptr_null
+; CHECK: select i1 %cmp.i, i32* %ptr, i32* null
+define void @test_select_trivial_ptr_null(i32* %ptr){
+entry:
+  %gv1_val = load i32, i32* %ptr, align 4
+  %cmp.i = icmp eq i32 %gv1_val, 0
+  %spec.select.i = select i1 %cmp.i, i32* %ptr, i32* null
+  br i1 %cmp.i, label %if.then, label %if.end
+
+if.then:
+  %val = load i32, i32* %spec.select.i, align 4
+  %inc = add nsw i32 %val, 1
+  store i32 %inc, i32* %spec.select.i, align 4
+  br label %if.end
+
+if.end:
+  ret void
+}
Index: lib/CodeGen/CodeGenPrepare.cpp
===================================================================
--- lib/CodeGen/CodeGenPrepare.cpp
+++ lib/CodeGen/CodeGenPrepare.cpp
@@ -2078,16 +2078,14 @@
       return static_cast<FieldName>(Result);
   }
 
-  // AddrModes with a baseReg or gv where the reg/gv is
-  // the only populated field are trivial.
+  // An AddrMode is trivial if it involves no calculation i.e. it is just a base
+  // with no offset.
   bool isTrivial() {
-    if (BaseGV && !BaseOffs && !Scale && !BaseReg)
-      return true;
-
-    if (!BaseGV && !BaseOffs && !Scale && BaseReg)
-      return true;
-
-    return false;
+    // An AddrMode is (BaseGV + BaseReg + BaseOffs + ScaleReg * Scale) so it is
+    // trivial if at most one of these terms is nonzero, except that BaseGV and
+    // BaseReg both being zero actually means a null pointer value, which we
+    // consider to be 'non-zero' here.
+    return !BaseOffs && !Scale && !(BaseGV && BaseReg);
   }
 
   Value *GetFieldAsValue(FieldName Field, Type *IntPtrTy) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40447.124228.patch
Type: text/x-patch
Size: 2992 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171124/4852bf07/attachment.bin>