[PATCH] D70413: [musttail] Don't forward incoming registers over call site registers
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 18 16:29:16 PST 2019
efriedma added inline comments.
================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:3901
+ // forwarding the original incoming value of AL. In these cases, the value
+ // passed at the call site wins.
const auto &Forwards = X86Info->getForwardedMustTailRegParms();
----------------
rnk wrote:
> efriedma wrote:
> > This doesn't seem right... either we need to forward the value, or we don't.
> >
> > We shouldn't be forwarding AL for Windows callling conventions; that convention for specifying the number of xmm arguments only applies to the "Linux" varargs ABI.
> So, I actually feel like musttail thunks really shouldn't assume a calling convention at all, they should simply preserve all non-CSR registers, including things like `RAX`, `R11`, `R10`, which are often scratch, and used for nest parameters, this CFG side parameter, and retpoline thunks. This makes it a much more general device for doing perfect forwarding. This change here is a step in that direction.
I guess my concern here, really, is that the "CCInfo.isAllocated" check seems sort of fragile? But I guess it's a fundamental part of the way the forwarding works; we have argument registers that may or may not be used by a known argument.
It still seems confusing if getForwardedMustTailRegParms() contains a register we know doesn't actually need to forward. And having a comment here describing that unnecessary functionality doesn't help.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70413/new/
https://reviews.llvm.org/D70413
More information about the llvm-commits
mailing list