From 8ed68b36716ac6bbe3c15beecdf2d9524874b4e1 Mon Sep 17 00:00:00 2001
From: John Brawn <john.brawn@arm.com>
Date: Mon, 27 Nov 2017 11:29:15 +0000
Subject: [PATCH] [CGP] Fix handling of null pointer values in
 optimizeMemoryInst

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 aware of null when deciding if an
addressing mode is trivial.

Differential Revision: https://reviews.llvm.org/D40447


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@319019 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/CodeGen/CodeGenPrepare.cpp                | 16 +++---
 .../CodeGenPrepare/ARM/sink-addrmode.ll       | 52 +++++++++++++++++++
 2 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/lib/CodeGen/CodeGenPrepare.cpp b/lib/CodeGen/CodeGenPrepare.cpp
index 64c5db9b560..75f9f81c112 100644
--- a/lib/CodeGen/CodeGenPrepare.cpp
+++ b/lib/CodeGen/CodeGenPrepare.cpp
@@ -2080,16 +2080,14 @@ struct ExtAddrMode : public TargetLowering::AddrMode {
       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) {
diff --git a/test/Transforms/CodeGenPrepare/ARM/sink-addrmode.ll b/test/Transforms/CodeGenPrepare/ARM/sink-addrmode.ll
index 0be9c556f93..a26edb19da0 100644
--- a/test/Transforms/CodeGenPrepare/ARM/sink-addrmode.ll
+++ b/test/Transforms/CodeGenPrepare/ARM/sink-addrmode.ll
@@ -366,3 +366,55 @@ entey:
   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
+}
-- 
2.50.1