[PATCH] D29478: [GlobalISel] Generate selector with predicates; use it for FP binops.

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

In https://reviews.llvm.org/D29478#666554, @ab wrote:

> In https://reviews.llvm.org/D29478#665821, @dsanders wrote:
>
> > This needs a testcase.
>
>
> Right!  I've been wanting to write tablegen tests forever but never remember.  I guess it just doesn't come naturally in these parts ;)  Done in r294075.


Sorry for being ambiguous here. I meant a test case that checks the output for G_FADD and the other newly-added FPU operations but having just checked test/CodeGen/AArch64/GlobalISel/arm64-instructionselect.mir it seems we're re-using some existing tests from when they were added to the C++ implementation.

Thanks for adding tablegen tests. I'll make sure my patches update them too.

> WIP: only recognize predicates that map to Subtarget features (the unfortunately named AssemblerPredicate). Those should always be supported.
> 
> I grabbed the feature flags off of the subtarget. We should do something similar to the asm matcher and cache the combined feature flags.
>  I'll look into deduplicating the various computeAvailableFeatures instances; we could also generate that code in the selector itself. Either way,
>  that gives us support for flag logic for free (e.g., "FeatureFoo,!FeatureBar").
> 
> In the meantime, what do you folks think?

IIRC, SelectionDAG only uses the Predicate class and the MC layer only uses AssemblerPredicate. It's probably safest to stick to that but there's some advantages to the AssemblerPredicate way so it makes sense to make use that approach in GlobalISel where possible. The main one I see is that it's possible to test multiple predicates simultaneously. If we do use AssemblerPredicates in GlobalISel, we shouldn't continue to call it AssemblerPredicate though.

I should mention that AArch64 has a few Predicates (e.g. IsLE) that don't have an AssemblerPredicate. We'll need to fix that. Also, Mips has an AssemblerPredicate that lacks a Predicate because it only makes sense to the assembly (UseTCCInDiv).


https://reviews.llvm.org/D29478