[PATCH] D67602: GlobalISel: Handle llvm.read_register

[Matt Arsenault via Phabricator via llvm-commits]

arsenm marked an inline comment as done.
arsenm 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())
----------------
rengolin wrote:
> 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?
Only through builtins and builtin library functions, not asm variables 


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

https://reviews.llvm.org/D67602