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
