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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
