[llvm] [CodeGen][X86] Fix lowering of tailcalls when `-ms-hotpatch` is used (PR #77245)

via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 8 07:14:17 PST 2024


sylvain-audi wrote:

I remember trying a similar approach: Making the pass skip the functions for which the first instruction's `isCall()` returns true, assuming that all calls/jumps are >= 2 bytes long.

One one hand, I think it fixed the issue for the repro provided in the 2 issues, so it could be considered a workaround (and it is not worse than before this code change).

However I don't think it should close https://github.com/llvm/llvm-project/issues/59039, because of the remaining design issues:

1) Wrapping an instruction into a `PATCHABLE_OP` loses information that would be needed at lowering time. 
In `X86AsmPrinter::emitInstruction`, the wrapped opcode doesn't go through `X86AsmPrinter::emitInstruction` itself, which  I'm pretty sure can become problematic. The main concern here is that `-fms-hotpatch` could alter the expected first instruction (instead of only guaranteeing its minimum size).

 2) `PATCHABLE_OP` takes the size as parameter (in reality it is always 2), but it is hard for `PatchableFunction` pass to know precisely the exact size of the instruction at that point. Especially, with your change, we start to assume that the size is 2. Maybe it should be changed to only handle a size of 2, and simplify the lowering code.


What do you think?

https://github.com/llvm/llvm-project/pull/77245


More information about the llvm-commits mailing list