[PATCH] D141980: [X86] Transform slow LEA_B_I_D/LEA_SLOWBASE_I -> LEA_IS_D/LEA_IS iff base == index

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added a comment.

In D141980#4062758 <https://reviews.llvm.org/D141980#4062758>, @craig.topper wrote:

> In D141980#4062735 <https://reviews.llvm.org/D141980#4062735>, @goldstein.w.n wrote:
>
>> In D141980#4061628 <https://reviews.llvm.org/D141980#4061628>, @RKSimon wrote:
>>
>>> An LEA with scale > 1 also forces a slower computation on the AGUs for most AMD cpus
>>
>> I see, is there a flag you know of that you think should guard the transform?
>>
>> Although keep in mind, this is only hooked in for targets that have `TuningSlow3OpsLEA` which AFAICT only covers:
>>
>>   KNL
>>   ICL
>>   ICX
>>   TGL
>>   CNL
>>   SKX
>>   CLX
>>   CPX
>>   SKL
>>   ADL
>>   HSW
>>   BDW
>>   SNB
>>   IVB
>>
>> Which are just the intel lineup. The only concerning one is `ADL` b.c. it has some `atom` cores and there where concerns `atom` has slow LEA_IS although looking at `ADL-E` on `uops.info` seems `LEA_IS_D` is same as `LEA_B_I_D` (2c latency each) so probably still fine to keep this transform (truthfully `ADL` probably shouldn't have `TuningSlow3OpsLEA` set as 2uops basically == 1x lea in worst case LEA on the E cores and the P cores have fast LEA).
>
> I find it odd that KNL is in that list. KNL is based on Silvermont so I'd expect it to have `TuningSlowLEA` instead.

We have:

  X86.td:L1104:
  ...
    list<SubtargetFeature> KNLTuning = [TuningSlowDivide64,
                                        TuningSlow3OpsLEA,
                                        TuningSlowIncDec,
                                        TuningSlowTwoMemOps,
                                        TuningPreferMaskRegisters,
                                        TuningFastGather,
                                        TuningFastMOVBE,
                                        TuningSlowPMADDWD];

Should it be changed to `TuningSlowLEA`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141980