[PATCH] D50636: [DAGCombiner] Add X / X -> 1 & X % X -> 0 folds.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

I'm happy to apply the test changes beforehand once everybody is happy with the patch - but I'll keep them in the diff for now to make its effects clear.



================
Comment at: test/CodeGen/SystemZ/pr32372.ll:18
   store i16 -3825, i16* undef
-  %L5 = load i8, i8* %0
+  %L5 = load volatile i8, i8* %0
   %B9 = urem i8 %L5, %L
----------------
jonpa wrote:
> craig.topper wrote:
> > jonpa wrote:
> > > RKSimon wrote:
> > > > Tried again against bleeding edge trunk and I still see this change and without the volatile (which prevents the loads being combined) the checks reduce to:
> > > > ```
> > > > define void @pr32372(i8*) {
> > > > ; CHECK-LABEL: pr32372:
> > > > ; CHECK:       # %bb.0: # %BB
> > > > ; CHECK-NEXT:    mvhhi 0(%r1), -3825
> > > > ; CHECK-NEXT:  .LBB0_1: # %CF251
> > > > ; CHECK-NEXT:    # =>This Inner Loop Header: Depth=1
> > > > ; CHECK-NEXT:    j .LBB0_1
> > > > ```
> > > Yes, I see the change after updating the test, but also note that this updated test passes also without the patch... So I am not sure why the test is changed if the patch does not affect it...
> > I think this patch affects the original version of the test. Without the volatile the two loads get CSEd to the same SDNode. This causes the same SDNode to be both operands of the urem. This patch would simplify that to 0. Simon added the volatile to prevent the loads from being CSEd. This way the urem won't get affected by this patch. So the new version should pass on trunk as well.
> Ah, I see. Thank you for the explanation.
Sorry - I missed that you were applying the test changes before the code changes!


Repository:
  rL LLVM

https://reviews.llvm.org/D50636