Page MenuHomePhabricator

[RISCV] Lower inline asm constraints I, J & K for RISC-V
ClosedPublic

Authored by lewis-revill on Nov 5 2018, 1:48 AM.

Details

Summary

This validates and lowers arguments to inline asm nodes which have the constraints I, J & K, with the following semantics (equivalent to GCC):

I: Any 12-bit signed immediate.
J: Immediate integer zero only.
K: Any 5-bit unsigned immediate.

Note that GCC also implements 'f' for floating point register and 'A' for address-only operand. These are not implemented here because:

  1. It appears trivial to implement the floating point register constraint, however since floating point registers are not yet recognised by the calling convention the call to the inline asm node cannot be lowered.
  1. I'm not yet certain how to implement an 'address-only' operand and I'd rather get the above constraints done first and add it later.

Diff Detail

Repository
rL LLVM

Event Timeline

lewis-revill created this revision.Nov 5 2018, 1:48 AM
lewis-revill planned changes to this revision.Nov 9 2018, 3:18 AM

Lowering the 'J' constraint to an immediate seems to be the wrong behaviour, as it should instead be put into the 'zero' register.

Use the register 'zero' for integer zero operands.

asb requested changes to this revision.Feb 14 2019, 5:06 AM

Did you check the behaviour of 'J' vs GCC, because for e.g. asm volatile ("sub a0, a0, %0" :: "J"(0)); I seem to see it using the integer 0 rather than the zero register?

I think it would be worth adding RV64I check lines to inline-asm.ll too.

This revision now requires changes to proceed.Feb 14 2019, 5:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 14 2019, 5:06 AM
In D54093#1397875, @asb wrote:

Did you check the behaviour of 'J' vs GCC, because for e.g. asm volatile ("sub a0, a0, %0" :: "J"(0)); I seem to see it using the integer 0 rather than the zero register?

I think it would be worth adding RV64I check lines to inline-asm.ll too.

Yes, the current behaviour of J in this patch is wrong, it should always be the constant integer 0, rather than the x0/zero register.

I think I ended up being thrown off at the time by the output of something with the '%z' modifier, which I see you have now implemented @jrtc27. I'll correct this patch.

Don't use zero register for 'J' constraint.

This probably needs some invalid tests (ie the out of range immediates mentioned in the TODO). Also, please run utils/update_llc_test_checks.py to get the RV64I test lines.

asb requested changes to this revision.Jun 6 2019, 5:47 AM

Hi Lewis - might you get a chance to update this with the changes suggested by @jrtc27? I made another minor comment too.

lib/Target/RISCV/RISCVISelLowering.cpp
1868 ↗(On Diff #187334)
This revision now requires changes to proceed.Jun 6 2019, 5:47 AM
lewis-revill edited the summary of this revision. (Show Details)
  • Updated llc test check lines
  • Add invalid constraint test
  • Address use of auto
asb accepted this revision.Jun 10 2019, 10:08 PM

LGTM, thanks for the updates!

This revision is now accepted and ready to land.Jun 10 2019, 10:08 PM
asb added inline comments.Jun 10 2019, 10:10 PM
lib/Target/RISCV/RISCVISelLowering.cpp
2168 ↗(On Diff #203364)

clang-format forms this as:

+          Ops.push_back(
+              DAG.getTargetConstant(0, SDLoc(Op), Subtarget.getXLenVT()));
This revision was automatically updated to reflect the committed changes.