llvm dev - Feb 2020 - [RFC] Extension to TableGen's AssemblerPredicates to support combining features with ORs

Hi,

I'd like to propose extending the supported syntax for
AssemblerPredicates to allow sets of SubtargetFeatures to be listed, but
where only one in the list has to be enabled for the predicate to be true.

The condition string which forms a AssemblerPredicate already allows
multiple features to be defined, separated by commas, and this means all
of these features must be present. For example, "Feature1,Feature2"
means that both Feature1 and Feature2 must be enabled in the Subtarget.

I propose extending this using the vertical bar character to mean or, so
eg "Feature1|Feature2" requires at least one of these two be enabled.
At
this stage I don't propose allowing ANDs and ORs to combine exist in the
same predicate, this can be revisited in the future if a need is found.

My initial motivation for this relates to the RISC-V backend and it's
upcoming bit manipulation extension. Internally this comprises of
several subextensions, each of which has its own SubtargetFeature. Some
of the instructions in this set require one of two of these target
features to be enabled for the instruction to be valid. Beyond this, I
can see such a feature will be useful elsewhere in the RISC-V backend as
different releases of the ISA would mean some instructions are enabled
or disabled based on one of a set of features being enabled, and such a
feature might be useful to other backends too.


I have implemented a prototype of this extension in
https://reviews.llvm.org/D74338. AssemblerPredicates are used in four
parts of TableGen, three of which only affect TableGen'erated code, and
one is RISC-V specific so these changes are not very intrusive. For
AsmWriterEmitter/MCInstPrinter to work with these changes, I've made
what I think may be a minimally distruptive change, but I'm happy to
hear any alternative ideas on how to express this to the AsmWriter alias
condition code.

I look forward to any ideas and feedback.

Thanks,
Simon

Hi Simon,

On Thu, Feb 13, 2020 at 7:13 PM Simon Cook via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> I'd like to propose extending the supported syntax for
> AssemblerPredicates to allow sets of SubtargetFeatures to be listed, but
> where only one in the list has to be enabled for the predicate to be true.
>
> The condition string which forms a AssemblerPredicate already allows
> multiple features to be defined, separated by commas, and this means all
> of these features must be present. For example,
"Feature1,Feature2"
> means that both Feature1 and Feature2 must be enabled in the Subtarget.
>
> I propose extending this using the vertical bar character to mean or, so
> eg "Feature1|Feature2" requires at least one of these two be
enabled. At
> this stage I don't propose allowing ANDs and ORs to combine exist in
the
> same predicate, this can be revisited in the future if a need is found.
This seems like an eminently reasonable feature to want to have.

I'm only worried that we're moving too far along the path of having
DSLs inside DSLs. I suppose the first step was already made when
adding comma-separated lists there, but perhaps we can still turn back
and using something more TableGen-y instead?

For example, rather than "AsmCond1,AsmCond2" or
"AsmCond1|AsmCond2",
could we perhaps allow dag expressions that look like `(and AsmCond1
AsmCond2)` and `(or AsmCond1 AsmCond2)`, respectively?

Cheers,
Nicolai

> My initial motivation for this relates to the RISC-V backend and it's
> upcoming bit manipulation extension. Internally this comprises of
> several subextensions, each of which has its own SubtargetFeature. Some
> of the instructions in this set require one of two of these target
> features to be enabled for the instruction to be valid. Beyond this, I
> can see such a feature will be useful elsewhere in the RISC-V backend as
> different releases of the ISA would mean some instructions are enabled
> or disabled based on one of a set of features being enabled, and such a
> feature might be useful to other backends too.
>
>
> I have implemented a prototype of this extension in
> https://reviews.llvm.org/D74338. AssemblerPredicates are used in four
> parts of TableGen, three of which only affect TableGen'erated code, and
> one is RISC-V specific so these changes are not very intrusive. For
> AsmWriterEmitter/MCInstPrinter to work with these changes, I've made
> what I think may be a minimally distruptive change, but I'm happy to
> hear any alternative ideas on how to express this to the AsmWriter alias
> condition code.
>
> I look forward to any ideas and feedback.
>
> Thanks,
> Simon
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev


-- 
Lerne, wie die Welt wirklich ist,
aber vergiss niemals, wie sie sein sollte.

Hi Nicolai,

On 15/02/2020 21:19, Nicolai Hähnle wrote:
> 
> This seems like an eminently reasonable feature to want to have.
> 
> I'm only worried that we're moving too far along the path of having
> DSLs inside DSLs. I suppose the first step was already made when
> adding comma-separated lists there, but perhaps we can still turn back
> and using something more TableGen-y instead?
> 
> For example, rather than "AsmCond1,AsmCond2" or
"AsmCond1|AsmCond2",
> could we perhaps allow dag expressions that look like `(and AsmCond1
> AsmCond2)` and `(or AsmCond1 AsmCond2)`, respectively?
I agree that having DSLs inside DSLs isn't ideal, and if I were
proposing adding more arbitrary combinations of operators this would be
a good way of expressing it.

I suppose we can go down that road now, and still keep the limitations
on the expressions already supported by the current approach. When I
next have time I'll put something together see how that
looks/works/feels and see which is the right route.

In the mean time does anyone have any other thoughts/preferences on
which way of expressing this seems most sensible?

Thanks,
Simon