[PATCH] D88287: [NARY-REASSOCIATE] Support reassociation of min/max

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri accepted this revision.
lebedev.ri added subscribers: nikic, spatel.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Please do add an precommit tests for pointers (unless i just missed them)

LG

In D88287#2618821 <https://reviews.llvm.org/D88287#2618821>, @ebrevnov wrote:

> In D88287#2609126 <https://reviews.llvm.org/D88287#2609126>, @lebedev.ri wrote:
>
>> Fixed SCEVExpaned in b46c085d2b6d15873fb53718f0a70b3848e19e4a <https://reviews.llvm.org/rGb46c085d2b6d15873fb53718f0a70b3848e19e4a>, please rebase/update this patch accordingly.
>>
>> But, i think we may have a problem still.
>> Does this patch intend to reassociate only integers, or pointers too? :)
>
> Updated to use SCEVExpander and limited to integer types only.

So i guess we may want to extend those intrinsics to support pointer types after all... @spatel @nikic



================
Comment at: llvm/lib/Transforms/Scalar/NaryReassociate.cpp:283-288
+  if (match(I, MinMaxMatcher)) {
+    OrigSCEV = SE->getSCEV(I);
+    return dyn_cast_or_null<Instruction>(
+        tryReassociateMinOrMax(I, MinMaxMatcher, LHS, RHS));
+  }
+  return nullptr;
----------------
Early return may be cleaner?


================
Comment at: llvm/lib/Transforms/Scalar/NaryReassociate.cpp:631
+        Value *NewMinMax = Expander.expandCodeFor(R2Expr, I->getType(), I);
+        NewMinMax->setName(Twine(I->getName()).concat(".nary"));
+
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88287