[PATCH] D108112: [LoopIdiom] let LIR fold memset pointer / stride SCEV regarding loop guards

Sat Aug 21 03:26:42 PDT 2021

eopXD added a comment.

In D108112#2957417 <https://reviews.llvm.org/D108112#2957417>, @bmahjour wrote:

> But what if we are compiling in 32-bit mode,

This is a valid point. Thank you for pointing this out.
Created D108507 <https://reviews.llvm.org/D108507> to add test case that is aware of 32-bit mode.

> or if 'n' and 'm' where 'signed long long'?

The accepted scenario for LIR is `MemsetSizeSCEV == `PositivePointerStrideSCEV`.
The `n, m` accepted would always be non-negative. I can't think of scenario when `n`, `m` needs to remain as a `signed long long`. (Please correct me if I'm wrong.

> I still have some reservations about the SCEVFolder implementation in this patch. It doesn't really fold anything, it just zero extends an expression if it is safe to do so. The assumption being made is that the expression it compares against is unsigned (so a "folding" opportunity is realized). In the LIT test provided the trip count is zero extended by loop simplify because it is originally 32-bit and the code is targeting 64-bit mode.

I agree with you that `SCEVFolder` has trivial functionality. It only to turn `sext` to `zext`. This patch only fixes the test case added, which is very limited. 
On the other hand, I don't think I have deep understanding to benchmarks. So I am not sure if I have covered most of the "practical memset idioms" that can be optimized here. Do you have other "fold loop-guard into SCEV-expression` cases that you wish this patch to have? I would be happy to resolve it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108112