This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Clear isCodeGenOnly flag on VMSGE(U) pseudo instructions. Remove InstAliases that duplicate the asm strings in the pseudos.
ClosedPublic

Authored by craig.topper on Jan 4 2021, 11:49 AM.

Details

Summary

The Pseudo class sets isCodeGenOnly=1 which causes the asm strings
in the pseudos to be ignored. I think this is why the aliases are
needed at all.

Diff Detail

Event Timeline

craig.topper created this revision.Jan 4 2021, 11:49 AM
craig.topper requested review of this revision.Jan 4 2021, 11:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 4 2021, 11:49 AM
Herald added a subscriber: MaskRay. · View Herald Transcript

Sorry, haven't felt able to review this as I don't get the original problem well enough. Because it looks to me like the aliases permitted matches, e.g. vmsgeu.vx $vd, $va, $rs1 which wasn't matching because it was unmasked? It's just that the comment says the "alias .. prevents matching" which throws me off as that's the opposite of what I thought they were for.

Sorry, haven't felt able to review this as I don't get the original problem well enough. Because it looks to me like the aliases permitted matches, e.g. vmsgeu.vx $vd, $va, $rs1 which wasn't matching because it was unmasked? It's just that the comment says the "alias .. prevents matching" which throws me off as that's the opposite of what I thought they were for.

I think the comment referred specifically to the first two aliases. The VMaskOp on the other patterns is allowed to match as either or ", v0.t" as we normally use the same instruction for masked and unmasked. But in this case we only want VMaskOp to match ", v0.t" so we need an explicit match to another instruction for the no mask case to "prevent" the VMaskOp pattern from matching it.

This patch doesn't change how that works. We're just using the strings from the instructions now instead of aliases.

frasercrmck accepted this revision.Jan 8 2021, 1:10 AM

Thanks for clarifying. LGTM.

This revision is now accepted and ready to land.Jan 8 2021, 1:10 AM