[PATCH] D51851: [CGP] Ensure splitgep gives deterministic output

[Dave Green via Phabricator via llvm-commits]

dmgreen created this revision.
dmgreen added reviewers: efriedma, haicheng, junbuml, mcrosier.
Herald added a reviewer: javed.absar.
Herald added subscribers: mgrang, kristof.beyls.

The output of splitLargeGEPOffsets does not appear to be deterministic because of the way that we iterate over a DenseMap. I've changed it to a MapVector for consistent output.

The test case here isn't particularly great. It shows a difference in output, but only really a cosmetic one. I have a (much) larger case that has the splitgep elements placed in a different order, leading to different registry allocation on Arm. It's the kind of problem goes away if I start removing comment out lines though :-/


https://reviews.llvm.org/D51851

Files:
  lib/CodeGen/CodeGenPrepare.cpp
  test/Transforms/CodeGenPrepare/ARM/splitgep.ll


Index: test/Transforms/CodeGenPrepare/ARM/splitgep.ll
===================================================================
--- /dev/null
+++ test/Transforms/CodeGenPrepare/ARM/splitgep.ll
@@ -0,0 +1,40 @@
+; RUN: opt -S -codegenprepare %s | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "thumbv6m-arm-none-eabi"
+
+; Check that we have deterministic output
+define void @test([65536 x i32]** %sp, [65536 x i32]* %t, i32 %n) {
+; CHECK-LABEL: @test(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    %0 = bitcast [65536 x i32]* %t to i8*
+; CHECK-NEXT:    %splitgep1 = getelementptr i8, i8* %0, i32 80000
+; CHECK-NEXT:    %s = load [65536 x i32]*, [65536 x i32]** %sp
+; CHECK-NEXT:    %1 = bitcast [65536 x i32]* %s to i8*
+; CHECK-NEXT:    %splitgep = getelementptr i8, i8* %1, i32 80000
+entry:
+  %s = load [65536 x i32]*, [65536 x i32]** %sp
+  br label %while_cond
+
+while_cond:
+  %phi = phi i32 [ 0, %entry ], [ %i, %while_body ]
+  %gep0 = getelementptr [65536 x i32], [65536 x i32]* %s, i64 0, i32 20000
+  %gep1 = getelementptr [65536 x i32], [65536 x i32]* %s, i64 0, i32 20001
+  %gep2 = getelementptr [65536 x i32], [65536 x i32]* %t, i64 0, i32 20000
+  %gep3 = getelementptr [65536 x i32], [65536 x i32]* %t, i64 0, i32 20001
+  %cmp = icmp slt i32 %phi, %n
+  br i1 %cmp, label %while_body, label %while_end
+
+while_body:
+  %i = add i32 %phi, 1
+  %j = add i32 %phi, 2
+  store i32 %i, i32* %gep0
+  store i32 %phi, i32* %gep1
+  store i32 %i, i32* %gep2
+  store i32 %phi, i32* %gep3
+  br label %while_cond
+
+while_end:
+  ret void
+}
+
Index: lib/CodeGen/CodeGenPrepare.cpp
===================================================================
--- lib/CodeGen/CodeGenPrepare.cpp
+++ lib/CodeGen/CodeGenPrepare.cpp
@@ -278,7 +278,7 @@
     /// Keep track of GEPs accessing the same data structures such as structs or
     /// arrays that are candidates to be split later because of their large
     /// size.
-    DenseMap<
+    MapVector<
         AssertingVH<Value>,
         SmallVector<std::pair<AssertingVH<GetElementPtrInst>, int64_t>, 32>>
         LargeOffsetGEPMap;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D51851.164633.patch
Type: text/x-patch
Size: 2143 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180910/dbf8eacd/attachment.bin>