[PATCH] D76518: [ARM] CMSE code generation

[Dave Green via Phabricator via llvm-commits]

dmgreen accepted this revision.
dmgreen added a comment.

LGTM still, whichever way you choose to go with the call site info.



================
Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1965
+        NewCall->addOperand(MI.getOperand(I));
+      MI.getMF()->moveCallSiteInfo(&MI, NewCall.getInstr());
+
----------------
chill wrote:
> dmgreen wrote:
> > chill wrote:
> > > This added line is the last change.
> > Do we need to call if (MI.isCandidateForCallSiteEntry())?
> I don't think so, we know exactly that we're replacing a `tBXNS_CALL`, for which `isCandidateForCallSiteEntry` ought to return true.
And do we know that will always remain true in the future? All other uses of moveCallSiteInfo I could find seemed to be guarded by isCandidateForCallSiteEntry, including the two in this file. You could argue that it should be moveCallSiteInfo's responsibility to check that, but what do you think of adding it here just for the sake of consistency?


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

https://reviews.llvm.org/D76518