Page MenuHomePhabricator

TableGen/GlobalISel: Partially fix nontrivial, custom predicates
ClosedPublic

Authored by arsenm on Jun 22 2020, 2:28 PM.

Details

Summary

Currently custom code predicates can only really be used for
contextless checks tied to a single instruction (e.g. check the def
for hasOneUse). If you do want to inspect the input instructions in
the source pattern, you cannot without re-verifying the opcode and
type checks implied by the patterns, since this check was emitted
before any operand constraints. Really, these are pattern level
predicates that implicitly depend on the instruction and operand
checks.

Introduce a filtering function so the custom predicate is emitted
last. I'm not sure this is the most elegant solution. It seems like
this is really a different thing from the InstructionMatcher/IPM_
predicate kinds. I initially tried keeping this in a separate
predicate list, but that also seemed awkward.

This only half fixes the problem I'm trying to solve. The AMDGPU
pattern I'm attempting to port also uses the PredicateCodeUsesOperands
feature to allow checks on the source operands when the input pattern
is commuted. Really the emitter should reject the pattern since it
doesn't handle this case, but at this point it would be more
productive to just implement this.

Diff Detail

Event Timeline

arsenm created this revision.Jun 22 2020, 2:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 22 2020, 2:28 PM
Herald added subscribers: tpr, rovka, wdng. · View Herald Transcript
arsenm updated this revision to Diff 272734.Jun 23 2020, 8:07 AM

Drop leftover change

I can't think of any better way to do this off the top of my head.

Maybe @dsanders has some opinions here?

llvm/utils/TableGen/GlobalISelEmitter.cpp
1048

I think it's worth documenting what the filter functions are for in a comment here.

1050–1052

Probably worth documenting in a comment too.

arsenm updated this revision to Diff 273851.Jun 26 2020, 3:52 PM

Add comments

paquette accepted this revision.Jul 13 2020, 10:12 AM

I think this is fine. LGTM

This revision is now accepted and ready to land.Jul 13 2020, 10:12 AM
dsanders accepted this revision.Jul 13 2020, 10:49 AM

Yeah, there isn't really a better way to do it as there's no dependency tracking in the importer. GIMatchDAG can track exactly what needs to be available for the predicate to be testable (making it just an issue of how to represent that in the source) but we aren't using that in the importer.