[PATCH] D37570: [XRay][CodeGen][PowerPC] Fix tail exit codegen for XRay in PPC

[Dean Michael Berris via Phabricator via llvm-commits]

dberris added inline comments.


================
Comment at: lib/CodeGen/XRayInstrumentation.cpp:118
-      }
-      if (TII->isTailCall(T)) {
-        Opc = TargetOpcode::PATCHABLE_TAIL_CALL;
----------------
timshen wrote:
> dberris wrote:
> > timshen wrote:
> > > dberris wrote:
> > > > timshen wrote:
> > > > > This is actively removing PATCHABLE_TAIL_CALL emits, and it affects all architectures.
> > > > > 
> > > > > Can we add x86 tests for tracking the decisions we've made?
> > > > Note that this function only gets called for non-X86 architectures (where prepending is the correct solution).
> > > > 
> > > > We already have tests in X86 for the tail call exit sled lowering. Those pass with this change.
> > > We have other non-x86 targets like aarch64, right? How do those targets test this?
> > Yes, we have the following tests aside from the ones we're adding here.
> > 
> > ```
> > test/CodeGen/ARM/xray-tail-call-sled.ll
> > test/CodeGen/AArch64/xray-tail-call-sled.ll
> > ```
> > 
> > Or did you mean we need a test for tail calls out to hidden/external functions?
> What does "out to" mean?
> 
> Yes. I was surprised that aarch64 tests are not affected by this patch. I guess that tests are missing and we should add them, but my guess can be wrong?
> What does "out to" mean?

I meant, since the callee is only declared and not defined in the same TU.

I suspect the aarch64 tests weren't affected, because `TII->isTailCall(T)` might not be true for the machine instruction that's used to represent the tail call.

I'm happy to add the tests later if that matters for aarch64, just so we can ensure that if anything changes in this codepath that might affect that target, that we can catch breakages of this form too.


https://reviews.llvm.org/D37570