This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Support machine constraint "S"
ClosedPublic

Authored by MaskRay on Jun 30 2021, 6:09 PM.

Details

Summary

Similar to D46745, "S" represents an absolute symbolic operand, which
can be used to specify the access models, e.g.

extern int var;
void *addr_via_asm() {
  void *ret;
  asm("lui %0, %%hi(%1)\naddi %0,%0,%%lo(%1)" : "=r"(ret) : "S"(&var));
  return ret;
}

'S' is documented in trunk GCC: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101275

Diff Detail

Event Timeline

MaskRay created this revision.Jun 30 2021, 6:09 PM
MaskRay requested review of this revision.Jun 30 2021, 6:09 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJun 30 2021, 6:09 PM

Hm, AArch64 handles ExternalSymbolSDNode too, but I don't see how you could ever end up with one...

llvm/test/CodeGen/RISCV/inline-asm-S-constraint.ll
8

These should be stripped

9

#1 doesn't exist

25

Label isn't needed

26

for consistency

54

Don't need the comment

55

Ditto

MaskRay updated this revision to Diff 355749.Jun 30 2021, 7:18 PM
MaskRay marked 6 inline comments as done.

scrub labels/comments

llvm/test/CodeGen/RISCV/inline-asm-S-constraint.ll
25

Omitting the entry label is allowed but having a label is canonical. Without it %0 needs to be %1.

jrtc27 added inline comments.Jun 30 2021, 7:32 PM
llvm/test/CodeGen/RISCV/inline-asm-S-constraint.ll
25

TIL

Hm, AArch64 handles ExternalSymbolSDNode too, but I don't see how you could ever end up with one...

It can't and the code path is untested. I already deleted it from aarch64.

Makes sense. Let's wait for the GCC Bugzilla feedback.

luismarques accepted this revision.Jul 12 2021, 8:29 AM

Makes sense. Let's wait for the GCC Bugzilla feedback.

With 'S' now documented on the GNU side, I think this can be merged.

Would we keep this constraint forever regardless of how discussions of [1] might evolve?

[1] https://github.com/riscv/riscv-elf-psabi-doc/issues/197

This revision is now accepted and ready to land.Jul 12 2021, 8:29 AM
MaskRay added a comment.EditedJul 12 2021, 10:20 AM

Makes sense. Let's wait for the GCC Bugzilla feedback.

With 'S' now documented on the GNU side, I think this can be merged.

Would we keep this constraint forever regardless of how discussions of [1] might evolve?

[1] https://github.com/riscv/riscv-elf-psabi-doc/issues/197

'S' will be kept forever, independent of issue/197. 'S' is useful on its own, for explicitly specifying the access modes by inline asm (I can image that the Linux kernel will use 'S' someday).
This will be useful even if we have an __attribute__ customizing the mode for regular C/C++ code: inline asm needs a way to access the symbol anyway.

MaskRay edited the summary of this revision. (Show Details)Jul 13 2021, 9:19 AM
jrtc27 added inline comments.Jul 13 2021, 9:24 AM
llvm/test/CodeGen/RISCV/inline-asm-S-constraint.ll
28

Not needed

This revision was automatically updated to reflect the committed changes.