This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU/GlobalISelemitter Support for predicate code that uses operands

Authored by Petar.Avramovic on Sep 8 2020, 6:33 AM.



Predicates with 'let PredicateCodeUsesOperands = 1' want to examine
matched operands. When we encounter predicate code that uses operands,
analyze its named operand arguments and create a map between argument
index and name. Later, when leaf node with name is encountered,
emit GIM_RecordNamedOperand that will store that operand at its
argument index in a operand list. This operand list will be argument
to c++ code of the predicate.

Diff Detail

Event Timeline

Petar.Avramovic created this revision.Sep 8 2020, 6:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 8 2020, 6:33 AM
Petar.Avramovic requested review of this revision.Sep 8 2020, 6:33 AM
arsenm added inline comments.Sep 8 2020, 7:15 AM

Can you be clearer on what "uses operands" means here?


Is this the recorded use or def operand? Can you add a comment here?

33 ↗(On Diff #290469)

Why arbitrarily resize this here?


Should update this comment


As a follow up, we need to do something about inline constants which will end up blocking this. I was thinking we need to start producing constants with VGPR bank with VALU uses prior to selection


Can you move the < %s to the end


Can you move the decrement out of the if?

Petar.Avramovic added inline comments.Sep 8 2020, 7:46 AM

As far as I know operands in this list are taken from instruction where they were use operands. Was that the question?

33 ↗(On Diff #290469)

Operands are inserted directly into specific index in RecordedOperands, see GIM_RecordNamedOperand.
Magic number 3 is the largest number of operands that any of the patterns with 'let PredicateCodeUsesOperands = 1' has.
I wanted to avoid having to keep additional maps between operand names and indices in final operand list that will be sent as argument to c++ predicate code' (like sdag). Unlike sdag, globalisel emitter first visits the predicate and we can figure out indices right away, sdag visits all operands first and predicate is visited last.

foad added inline comments.Sep 8 2020, 8:55 AM

Can't this be an ArrayRef<MachineOperand *> ? (Or a pointer to const std::array ?)

33 ↗(On Diff #290469)

If you really want a fixed size array, wouldn't std::array be better?

Add more verbose comments, and switch to std::array as container for operands.

arsenm added inline comments.Sep 9 2020, 7:39 AM

Can we put this string generation in a function somewhere? I think this is repeated 2 or 3 times

Factor string generation of operand names into function.

arsenm added inline comments.Sep 10 2020, 4:44 PM

Should assert StoreIdx < array size


Alternatively, tablegen could error if it hits the hardcoded limit?

Add assert for StoreIdx.

arsenm accepted this revision.Sep 11 2020, 2:13 PM

LGTM, but a tablegen error would be more discoverable when somebody hits the limit

This revision is now accepted and ready to land.Sep 11 2020, 2:13 PM