[PATCH] D62897: [DAGCombine] Initialize the bytes offset vector as INT64_MAX to avoid inference the endian check

[Qing Shan Zhang via Phabricator via llvm-commits]

steven.zhang created this revision.
steven.zhang added reviewers: RKSimon, apilipenko, lebedev.ri, jsji, nemanjai, hfinkel, kbarton.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

This is a regression of https://reviews.llvm.org/D61843 found by the build bot. This is the narrow down case:

  void foo(char *p, char v) {
          p[0] = v;
          p[1] = v;
  }

We have a table ByteOffsets to map the offset in the combined value "v"(the vector index) to the offset in the store address "p" (the vector element value).

In this case, we will have:

  ByteOffsets[0] = 0;     // The first byte of "v" is stored to p[0]              
  ByteOffsets[0] = 1;     // The first byte of "v" is stored to p[1]

Therefore, at last, ByteOffsets[0] will be set as "1". However, as this vector is created with 2 elements that has "0" as its element default value, therefore, it will look like this:

  ByteOffsets[0] = 1; // As described before.
  ByteOffsets[1] = 0; // The ctor initialized it as 0.

And this match the endian check, so, we will do the transformation, which is wrong.

Therefore, we need to initialize the default value of the table as INT64_MAX to avoid this issue. In fact, the range of the value is, [-63, 63], as it is offset from the base ("p"). And we can only merge 64 stores at most.

I have run the check-all, llvm testsuite, spec2017, and bootstrap, all are clean.


https://reviews.llvm.org/D62897

Files:
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/test/CodeGen/PowerPC/store-combine.ll


Index: llvm/test/CodeGen/PowerPC/store-combine.ll
===================================================================
--- llvm/test/CodeGen/PowerPC/store-combine.ll
+++ llvm/test/CodeGen/PowerPC/store-combine.ll
@@ -574,3 +574,23 @@
   store i8 %conv5, i8* %arrayidx6, align 1
   ret void
 }
+; This was found when testing the hexxagon in testsuite
+; i8* p; i8 v;
+; p[0] = v;
+; p[1] = v;
+define void @store_same_value_to_consecutive_mem(i8* %p, i8 zeroext %v) {
+; CHECK-PPC64LE-LABEL: store_same_value_to_consecutive_mem 
+; CHECK-PPC64LE:       # %bb.0: # %entry 
+; CHECK-PPC64LE-NEXT:    stb 4, 0(3) 
+; CHECK-PPC64LE-NEXT:    stb 4, 1(3) 
+;
+; CHECK-PPC64-LABEL: store_same_value_to_consecutive_mem 
+; CHECK-PPC64:       # %bb.0: # %entry
+; CHECK-PPC64-NEXT:    stb 4, 0(3)
+; CHECK-PPC64-NEXT:    stb 4, 1(3)
+entry:
+  store i8 %v, i8* %p, align 1
+  %arrayidx1 = getelementptr inbounds i8, i8* %p, i64 1
+  store i8 %v, i8* %arrayidx1, align 1
+  ret void
+}
Index: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -6304,7 +6304,7 @@
   // to the same base address. Collect bytes offsets from Base address into 
   // ByteOffsets. 
   SDValue CombinedValue;
-  SmallVector<int64_t, 4> ByteOffsets(Width);
+  SmallVector<int64_t, 4> ByteOffsets(Width, INT64_MAX);
   int64_t FirstOffset = INT64_MAX;
   StoreSDNode *FirstStore = nullptr;
   Optional<BaseIndexOffset> Base;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D62897.203110.patch
Type: text/x-patch
Size: 1555 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190605/dbee8c9b/attachment.bin>