[llvm] [GlobalISel] Add boolean predicated legalization action methods. (PR #111287)

David Green via llvm-commits llvm-commits at lists.llvm.org
Sat Oct 12 02:11:16 PDT 2024


davemgreen wrote:

> > For the new `legalFor` and `customFor`, I have a slight preference for replacing `For` with `If` . Then it is clear that the first parameter is a predicate and not a type.
> 
> We already have a predicated `legalIf` which takes a legality predicate.
> 
> I'm not sure of this change. I get the motivation but I'm wondering if we have a better way to express it than having to plumb in a boolean into specific rules. Perhaps a guard wrapper or something so we can write:
> 
> ```
> AddIf(Cond, legalFor({s32, s64}))
> ```
> 
> and that way it generalizes to all of the rule methods.

The was one thing I thought of, yeah. But I'm not sure how it would work. Do you know how that would work in C++, how would it pass the legalFor method through to the AddIf, it they are both part of the LegalizeRuleSet?

> Adding one more idea - I attempted to propose a predicate `hasFP16()` to be used in combination of `all`. So a typical usage looks like:
> 
> ```c++
> .legalIf(all(hasFP16(), typeInSet(0, {v8s16, v4s16})))
> ```
> 
> Which sounds like @aemerson 's idea but does not require a new `AddIf`.
> 
> See [#104753 (comment)](https://github.com/llvm/llvm-project/pull/104753#discussion_r1731827259)

Yep that was what I was trying to avoid. I wrote this back when that patch was created (hence my suggestion to remove it), but rewrote it again recently. IMO it will get very ugly if we have to use the low-level interface for everything, and this is a common enough issue that having a specific interface for it is probably worth while. It should also at least in theory be a little quicker if it doesn't need to store the hasFP16 predicate and keep checking it again and again.

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


More information about the llvm-commits mailing list