[llvm] [SelectOpt] Support ADD and SUB with zext operands. (PR #115489)

David Green via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 13 07:56:15 PST 2024


davemgreen wrote:

> I am currently working on enabling it for Apple CPUs and for some motivating cases we need support for `Adds`. The motivating cases are somewhat similar to the ones mentioned in #115745.

That's good to hear. I was under the impression (without knowing any of the details) that they were at least less susceptible to the issues we see. Maybe they are present everywhere to some extent.

> Overall enabling it with this change is neutral on SPEC and some other proprietary benchmarks. I agree that there is the risk that if we get it wrong the result can be pretty bad. But I don't think the `Add` case is different to the `Or` case and if anything the cost-model should be responsible for making the decision.

Yeah the add and or are very similar. We ruled out logical and/or in ca78b5601466f8515f5f958ef8e63d787d9d812e, and recently prevented it from folding selects with two constant operands in 1789534ac1ac95f9a3369625e331b9a446be60d9. I also tried to make the or relatively constrained when I added it, only adding them where a branch already existed (a2d68b4bece54bcfa7bfde1e80058ab19b6d1775). I am a bit suspicious of things that look like logic or arithmetic, as they are sometimes quicker as logic/arithmetic.

The problem with the cost model is that it doesn't know how predictable the branch is, and so doesn't have full info available. My understanding is that well-predictable loops can benefit greatly from converting the selects, but in larger codebases that are already heavy on the number of branches it can introduce extra overhead.

> I expect that as we support more cases, we are likely hitting additional cases where the cost-model gets things wrong. What do you think is the best way to make progress here?

Honestly - My hope was that they would fix it in the hardware :) I ran this and it didn't look too bad, but I didn't look into any specific case deeply. 

Do you have tests for commuted sub operands, where the first operand is the condition?

https://github.com/llvm/llvm-project/pull/115489


More information about the llvm-commits mailing list