This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Avoid selecting XZR inline ASM memory operand
ClosedPublic

Authored by kongyi on Jul 5 2017, 1:23 AM.

Details

Summary

Restricting register class to PointerRegClass for memory operands.

Also fix the PointerRegClass for AArch64 from GPR64 to GPR64sp, since XZR cannot hold a memory pointer while SP is.

Fixes PR33134.

Diff Detail

Repository
rL LLVM

Event Timeline

kongyi created this revision.Jul 5 2017, 1:23 AM

Is this really the best place to solve this issue?

PowerPC has a similar problem (r0 can't be used for memory addresses); the PPC backend solves it in PPCDAGToDAGISel::SelectInlineAsmMemoryOperand.

For non-memory inline asm operands, SelectionDAGBuilder::visitInlineAsm has code to constrain the register classes.

lib/CodeGen/SelectionDAG/InstrEmitter.cpp
161 ↗(On Diff #105226)

Let me see if I follow this correctly. At this point, either isMachineOpcode is true, or the opcode is one of the ones listed in the switch in InstrEmitter::EmitSpecialNode (ISD::MERGE_VALUES, ISD::TokenFactor, ISD::CopyToReg, ISD::CopyFromReg, ISD::EH_LABEL, ISD::LIFETIME_START, ISD::LIFETIME_END, and ISD::INLINEASM). For machine opcodes, we use getRegClass() to get the register class. For inline asm, we use this new code (roughly equivalent to MachineInstr::getRegClassConstraint) to get the register class. For the other opcodes, the register class doesn't matter...?

InstrEmitter::AddRegisterOperand has similar code to retrieve a regclass for an operand; do we also need to special-case inline asm there?

kongyi updated this revision to Diff 106704.Jul 14 2017, 2:07 PM
kongyi retitled this revision from [CodeGen] InstrEmitter should constrain register class for inline ASM to [AArch64] Avoid selecting XZR inline ASM memory operand.
kongyi edited the summary of this revision. (Show Details)

Is this really the best place to solve this issue?

PowerPC has a similar problem (r0 can't be used for memory addresses); the PPC backend solves it in PPCDAGToDAGISel::SelectInlineAsmMemoryOperand.

For non-memory inline asm operands, SelectionDAGBuilder::visitInlineAsm has code to constrain the register classes.

Thanks, this is a more suitable place to implement the logic.

efriedma accepted this revision.Jul 14 2017, 2:22 PM

LGTM, with one minor comment.

test/CodeGen/AArch64/asm-zero-address.ll
9 ↗(On Diff #106704)

Maybe put the test in test/CodeGen/AArch64/arm64-inline-asm.ll instead of its own file?

This revision is now accepted and ready to land.Jul 14 2017, 2:22 PM
This revision was automatically updated to reflect the committed changes.
kongyi marked an inline comment as done.Jul 14 2017, 2:47 PM