[PATCH] D109789: [SCEV] Stop blindly propagating flags from inbound geps to SCEV nodes

[Nikita Popov via Phabricator via llvm-commits]

nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

LGTM in that the code change here is clearly right. If we get too many perf regression reports though, we may have to back this out and do some improvements to the poison reasoning first. I don't consider fixing this particularly urgent, as it's a very long-standing known bug.

Note that there are some polly test failures -- please update those as well to make sure there are no surprises there.



================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/memcheck-wrapping-pointers.ll:51
 ; CHECK-NEXT:    [PSE]  %arrayidx4 = getelementptr inbounds i32, i32* %b, i64 %conv11:
-; CHECK-NEXT:      ((4 * (zext i32 {0,+,1}<%for.body> to i64))<nuw><nsw> + %b)<nuw>
+; CHECK-NEXT:      ((4 * (zext i32 {0,+,1}<%for.body> to i64))<nuw><nsw> + %b)
 ; CHECK-NEXT:      --> {%b,+,4}<%for.body>
----------------
I believe we're missing out here, because isSCEVExprNeverPoison() only looks at the immediate operations of the SCEV to find an addrec. In this case, it would have to look through the mul and the zext to find it.


================
Comment at: llvm/test/Analysis/LoopCacheAnalysis/PowerPC/stencil.ll:15
+; CHECK-DAG: Loop 'for.i' has cost = 20300
+; CHECK-DAG: Loop 'for.j' has cost = 700
 
----------------
Lower cost after less precise analysis seems suspicious...


================
Comment at: llvm/test/Analysis/ScalarEvolution/load.ll:76
 ; CHECK-NEXT:    %i = getelementptr inbounds %struct.ListNode, %struct.ListNode* %n.01, i64 0, i32 1
-; CHECK-NEXT:    --> (4 + %n.01)<nuw> U: [4,0) S: [4,0) Exits: (4 + @node1)<nuw><nsw> LoopDispositions: { %for.body: Variant }
+; CHECK-NEXT:    --> (4 + %n.01) U: full-set S: full-set Exits: (4 + @node1)<nuw><nsw> LoopDispositions: { %for.body: Variant }
 ; CHECK-NEXT:    %0 = load i32, i32* %i, align 4
----------------
We should be able to keep this one by considering not just addrecs, but also SCEVUnknown defined in the loop header.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109789/new/

https://reviews.llvm.org/D109789