SjoerdMeijer added inline comments.
================ Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:1495 let DecoderNamespace = "Fallback"; + let Defs = [NZCV]; } ---------------- 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. 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