dmgreen added inline comments.
================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:15993
+ DAG.getConstant(0, dl, MVT::i32),
+ DAG.getConstant(AArch64CC::EQ, dl, MVT::i32), A.getValue(1));
+ return DAG.getMergeValues(
----------------
Can you make sure EQ is correct here? I think it might be backwards.
================
Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:1495
let DecoderNamespace = "Fallback";
+ let Defs = [NZCV];
}
----------------
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.
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
