[PATCH] D67602: GlobalISel: Handle llvm.read_register

[Renato Golin via Phabricator via llvm-commits]

rengolin added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/IRTranslator.cpp:1529
+    EVT VT = TLI->getValueType(*DL, CI.getType());
+    Register Reg = TLI->getRegisterByName(RegStr->getString().data(), VT, *MF);
+    if (!Reg.isValid())
----------------
arsenm wrote:
> paquette wrote:
> > rengolin wrote:
> > > paquette wrote:
> > > > `TLI->getRegisterByName` can't handle sysregs on the AArch64 side, since they're modelled as immediate operands. So, this approach can't work for AArch64.
> > > > 
> > > > (see llvm/test/CodeGen/AArch64/special-reg.ll for an example)
> > > By design, they can only get the stack pointer, I think on any target. It should be an assembler error otherwise.
> > I don't think that's true in AArch64. E.g.:
> > 
> > https://reviews.llvm.org/D53115
> > 
> > The test added there uses read/write_register to access other special registers. This change made the test added there fail.
> > 
> > There's also the special-reg.ll test I mentioned above which has stuff like this:
> > 
> > ```
> > %reg = call i64 @llvm.read_register.i64(metadata !1)
> > ...
> > !1 = !{!"daif"}
> > ```
> > 
> > Which is supposed to produce this (which does not involve SP):
> > 
> > ```
> > mrs x0, DAIF
> > ```
> > 
> > So I don't think that the stack pointer is the only thing we have to think about here. (Unless I'm seriously misunderstanding something, which is entirely possible. :P)
> > 
> We use this for the EXEC mask (with the special consideration that the call site needs to be marked convergent)
Is that exposed to the users? For example, by declaring a register variable asm("daif")?

Writing to such a "variable" would be potentially dangerous, no?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67602/new/

https://reviews.llvm.org/D67602