SjoerdMeijer added inline comments.
================ Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:1495 let DecoderNamespace = "Fallback"; + let Defs = [NZCV]; } ---------------- SjoerdMeijer wrote: > dmgreen wrote: > > SjoerdMeijer wrote: > > > dmgreen wrote: > > > > SjoerdMeijer wrote: > > > > > Do all MRS instructions do this? > > > > No, but some do and it's not obvious which ones do and don't. I think > > > > it should be reasonable to always def NZCV here, even if they are just > > > > dead. It should be very rare that it would be beneficial for NZCV to be > > > > live across a MRS instruction. > > > True, but since creating another definition is cheap, what would the cons > > > be of: > > > > > > class MRS_NZCV : MRSI { > > > .. > > > let Defs = [NZCV]; > > > } > > > > > > The way I look at it is that the description would be more accurate? > > I believe that would have an over-lapping definition with the existing MRS > > instruction? > > > > It would need to be a pseudo I think, which would eventually be turned into > > a MSR proper late enough on the pipeline for it to be valid (or the other > > way around, the no-nzcv version gets turned into a the nzcv version to keep > > the verifier happy). > > > > It could also be a optional def, but those are only used in the arm backend > > and I would not recommend using them anywhere else. I would probably > > suggest just setting MRS as a NZCV setting instruction, unless we find a > > reason to need to implement it differently. > > I believe that would have an over-lapping definition with the existing MRS > > instruction? > > Ah yeah, that might be true. > > > It would need to be a pseudo I think > > How much work is adding a pseudo for this case? My first reaction would be > just trying to model this correctly, then we potentially don't have to worry > again later about this. I just wanted to add that I don't have too strong opinions on this, but what I suggested seemed more the correct, even though the consequence of setting NCZV for all MRS is minimal. So I will leave this one up to you @dmgreen and @stelios-arm . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98264/new/ https://reviews.llvm.org/D98264 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits