[PATCH] D141243: [SCEV] Add llvm.experimental.guard conditions to applyLoopGuards()

Mon Jan 9 10:04:15 PST 2023

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

In D141243#4036908 <https://reviews.llvm.org/D141243#4036908>, @caojoshua wrote:

> In D141243#4035889 <https://reviews.llvm.org/D141243#4035889>, @nikic wrote:
>
>> We should add support for guard intrinsics to AssumptionCache instead. For all practical purposes, they need the same handling, and it makes little sense that we keep implementing separate handling for assumes and guards each time.
>
> I agree with this thought, but this adds a lot of discussion and investigation to a simple PR. I've created https://github.com/llvm/llvm-project/issues/59901 to discuss this issue (can you leave your thoughts in the thread?), and I would not mind personally working on it.
>
> In my opinion, we can still land this review, and work on this as a separate issue. Does this make sense?

Sure, as long as someone is working on fixing this properly, we can land this in the meantime. LGTM

================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:15024
         if (const SCEVUnknown *LHSUnknown = dyn_cast<SCEVUnknown>(URemLHS)) {
-          auto Multiple = getMulExpr(getUDivExpr(URemLHS, URemRHS), URemRHS);
+          const auto *Multiple = getMulExpr(getUDivExpr(URemLHS, URemRHS), URemRHS);
           RewriteMap[LHSUnknown] = Multiple;
----------------
`const SCEV *`

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141243