[PATCH] D145370: [AArch64] Fix N2 SchedModel for arithmetic and logic ops with cheap LSL

Fri Mar 10 04:07:40 PST 2023

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

LGTM. Thanks

================
Comment at: llvm/lib/Target/AArch64/AArch64SchedPredNeoverse.td:17
+// Check for LSL shift <= 4
+def NeoverseCheapLSL : MCSchedPredicate<
+                         CheckAll<
----------------
rjj wrote:
> dmgreen wrote:
> > This is a quite common pattern and I think already exists somewhere. Can you move it somewhere shared?
> I believe you are referring to the Exynos and Ampere versions? Though similar in spirit, they actually implement a slightly different logic (I'm not sure if intentionally or not). In particular, the Exynos considers a "cheap shift" any shift = 0, or LSL <= 3. The Ampere considers cheap shifts to be any shift = 0, or LSL <= 4. As far as I could test, this is not accurate for the N2. For example, on N2 an LSR = 0 is still an "expensive shift". Since I was not sure if these slight differences were intentional, I decided to create a separate predicate for the Neoverses.
> 
> Having said that, I agree it would be good to move it to a shared location! Do you have any suggestions as to where that could be?
Ah I see. It was the Ampere version I was thinking of. I would guess a lsr with a shift of 0 isn't too important, and combining the two wouldn't be a problem in practice, but if they are different then that's fine.

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

https://reviews.llvm.org/D145370