[clang] [llvm] [RISCV] Inline Assembly Support for GPR Pairs ('Pr') (PR #112983)

Sam Elliott via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 29 07:12:17 PDT 2024


https://github.com/lenary commented:

> Thanks! I just had a detailed look. Given that you have explained almost all the code detailedly, I think this PR looks great to me! Just some overall comments:
> 
> 1. I personally like your proposal of adding new constraints, but we still need the agreement between community members.

I think some of the agreement needs an implementation, maybe? I'll ping the c-api issue.

> 2. I saw all the comments above and I know reason why we choose to add new MVT types. My question is, maybe we can make it less target-specific? I don't think this is a RISC-V only problem.

I noticed, after-the-fact, that SystemZ uses `MVT::Other` for pairs, I can prepare a fixup commit that does this (and removes the `riscv_*_pair` MVTs), if we think that's a better target-independent approach?

> 3. We should be able to use `Pr` for Zacas now? So maybe we should add some tests for it.

- The test functions which have an in-out `Pr` parameter (`test_Pr_wide_scalar_inout`) are derived from the issue about `amocas.q` in inline assembly on RV64. The C from that issue found some bugs in the implementation, which is why those tests exist. These don't reference any specific instructions just to make them have less complex dependencies - which is why the text of the inline assembly is just a comment to verify we're producing sensible output.
- The Zacas pair instructions continue to use `GPRPair` in their definitions, rather than the new `GPRF64Pair` register classes, which seemed the right way forwards. There are no patterns for the pair instructions at the moment, but I'm not sure I want to change the codegen of atomics, that's a whole other nightmare. Do you have specific tests in mind?

https://github.com/llvm/llvm-project/pull/112983


More information about the cfe-commits mailing list