This is an archive of the discontinued LLVM Phabricator instance.

[SPARC][IAS] Add SETX pseudoinstruction
ClosedPublic

Authored by koakuma on Aug 6 2023, 7:09 AM.

Details

Summary

This adds the v9 SETX pseudoinstruction for convenient loading of 64-bit values.

Depends on D144936

Diff Detail

Event Timeline

koakuma created this revision.Aug 6 2023, 7:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 6 2023, 7:09 AM
koakuma requested review of this revision.Aug 6 2023, 7:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 6 2023, 7:09 AM
barannikov88 added inline comments.Aug 6 2023, 12:40 PM
llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
660

Looks like dead code (MatchInstructionImpl should return Match_MissingFeature). Can you add a test for this as in D144936?

663
668

This will be executed if the operand is an expression. This is probably not desired. Please add a test.

671
llvm/lib/Target/Sparc/SparcInstrAliases.td
449–451

Wrapped lines should be aligned after '<'.

koakuma updated this revision to Diff 548211.Aug 8 2023, 7:22 AM
koakuma marked 4 inline comments as done.

Fix lowering of 32-bit constant expression & improve formatting.

koakuma added inline comments.Aug 8 2023, 7:25 AM
llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
660

It should be unreachable, yes, but for some reason it still gets through even though I've already specified Requires<[Is64Bit]> in the SETX definition.
Any idea how to properly fix this?

barannikov88 added inline comments.Aug 8 2023, 9:43 AM
llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
660

Requires<[HasV9]> should work (the predicate must be derived from AssemlerPredicate).

barannikov88 added inline comments.Aug 8 2023, 10:03 AM
llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
673

Can this be delegated to expandSET, too? It appears to have some logic related to handling of small negative numbers.

koakuma updated this revision to Diff 549693.Aug 13 2023, 6:05 AM

Add HasV9 in the Requires list.

llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
673

Unfortunately no, on 64-bit mode set is defined to zero the upper 32-bits of the destination register.

barannikov88 accepted this revision.Aug 15 2023, 4:24 AM

LGTM with one suggestion.

llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
665

It would be cleaner to inline the necessary pieces of expandSET here, probably combining with the other if. It would also avoid relying on the same order of operands of SET and SETX.

This revision is now accepted and ready to land.Aug 15 2023, 4:24 AM
koakuma updated this revision to Diff 552382.Aug 22 2023, 8:25 AM

Don't use expandSET anymore and further simplify code.

This revision was automatically updated to reflect the committed changes.