Analyzing symbols defined/references in the inline assembly (Bug 28970), taking into account module/file-scope inline assembly.
Provide some information about the cost of the inline assembly? (I’m not sure if we want to couple it with this issue and if the cost should be represented in this new IR or some other way)
This new IR should contain:
A list of symbols referenced by the inline assembly
a. Should we try to provide better analysis of how we use these symbols (e.g. in jmp instructions, in call instructions)? Can it help us do some error checking?
A list of symbols defines by the inline assembly
a. Should we provide additional information about how symbols defined (e.g. which symbols defined as globals)?.
At a later stage we also need to discuss other aspects of this new instruction (e.g. it being a block terminator and other behaviors we might want to be affected by it).
Any other thoughts and ideas we want to include in this change that I’m missing?
I wanted to revive this issue of supporting asm goto (Bug 9295).
FWIW, my opinion is that the use cases don’t really justify the complexity this adds to the compiler. I feel like embedding inline assembly into normal C control flow constructs and writing the entire thing directly give pretty reasonable options.
But maybe others really see high-value reasons to add support for this… If they do…
As was already proposed, the best way seems to be introducing new IR.
If we’re changing the IR, we should probably provide an infrastructure that solves or at least enables future support for things like:
I strongly agree with the ‘WONTFIX’ resolution here. More than GCC’s “asm goto”, this feature seems much more harmful to the compiler and much less well motivated.
Analyzing symbols defined/references in the inline assembly (Bug 28970), taking into account module/file-scope inline assembly.
Unless we decide to address #1 in some way, this seems completely orthogonal to “asm goto”. While I’d love to see a good resolution to PR28970, I don’t think it makes sense to couple the two together. Among other things, none of my concerns about “asm goto” apply to simply exposing the symbols defined.
Provide some information about the cost of the inline assembly? (I’m not sure if we want to couple it with this issue and if the cost should be represented in this new IR or some other way)
I suspect this too should be an orthogonal discussion.
I think we should expose a TTI-cost-analysis API and allow passing inline assembly to it. Then targets can, if they choose, actually analyze the assembly to compute a cost. But I don’t think we need IR changes here and even if we do, likely orthogonal ones to “asm goto”.
Linux kernel is using the “asm goto” feature, other projects probably use it as well. I think it provides motivation to support it in LLVM.
Regarding the complexity, I believe there is some infrastructure that we can at least partially reuse (the support for “indirectbr” instruction).
My focus is adding “asm goto” support, the other things are indeed completely orthogonal and came up in bugs related to Bug 9295.
The reason I mentioned them in this discussion is that they seem to require IR change as well.
I thought that if we’re doing this IR change for “asm goto” it’s worth checking if there is additional useful information we want to expose except the C labels used by inline assembly.
If you prefer to separate the IR changes for “asm goto” and the IR changes that will allow exposing defined symbols, then we can focus on the “asm goto” feature alone.
Just responding to the motivation stuff as that remains an open question:
Linux kernel is using the “asm goto” feature,
But your original email indicated they have an alternative code path for compilers that don’t support it?
What might be compelling would be if there are serious performance problems when using the other code path that cannot be addressed by less invasive (and more general) improvements to LLVM. If this is the only way to get comparable performance from the Linux Kernel, then I think that might be an interesting discussion. But it would take a very careful and detailed analysis of why IMO.
other projects probably use it as well.
This is entirely possible, but I’d like to understand which projects and why they use it rather than any of the alternatives before we impose the implementation complexity on LLVM. At least that’s my two cents.
As you’ve highlighted, LLVM’s support for inline assembly could definitely use work. It is not a well-loved area of the compiler. =/ These are some of my thoughts on what we could do.
I’d prefer to not support jumps into and out of MS inline asm blobs. I sketched out a design for doing it in http://llvm.org/pr24529#c4, but it creates more LLVM IR basic blocks that have no valid insertion point. I’d like to move away from that, not towards it. MSVC doesn’t even support inline asm on non-x86, so over time there will be less demand for this esoteric feature, not more.
I actually think supporting a terminator asm instruction for “asm goto” wouldn’t be that expensive. We already have terminators like indirectbr and catchswitch that create edges that can’t be split. I suspect we will want to add more to support parallel IR. Adding one for “asm goto” doesn’t seem that bad, but I’m not convinced it’s worth it.
Digging the symbol uses out of inline asm and the symbol definitions out of module inline asm seems totally reasonable. Adding this to the IR representation seems nice.
Improving our cost estimates with the help of MC would also be nice.
Asm goto feature was introduces to GCC in order to optimize the support for tracepoints in Linux kernel (it can be used for other things that do nop patching).
GCC documentation describes their motivating example here:
Asm goto feature was introduces to GCC in order to optimize the support for tracepoints in Linux kernel (it can be used for other things that do nop patching).
GCC documentation describes their motivating example here:
In this example (which in fact inspired the asm goto feature) we want on rare occasions to call the trace function; on other occasions we’d like to keep the overhead to the absolute minimum. The normal code path consists of a single nop instruction. However, we record the address of this nop together with the address of a label that calls the trace function. This allows the nop instruction to be patched at run time to be an unconditional branch to the stored label. It is assumed that an optimizing compiler moves the labeled block out of line, to optimize the fall through path from the asm.
Here is the Linux kernel RFC which discusses the old C way of implementing it and the performance issues that were noticed.
It also states some performance numbers of the old C code vs. the asm goto:
This presentation also mentions that a lot of other Linux applications use this tracing mechanism.
Thanks, this is exactly the kind of discussion that I think will help make progress here.
I think this feature makes a lot of sense and is a really nice feature. However, I think implementing it with inline assembly imposes a lot of really unfortunate constraints on compilation – it requires asm goto, pushsection and popsection, etc.
I would much rather provide a much more direct way to represent a patchable nop and the addresses of label within a function. For example, I could imagine something like:
if (0) { trace_call: /* code to call the trace function */ }
patch: __builtin_patchable_nop()
__builtin_save_labels(trace_call, patch)
But someone can probably design a much better way to represent this in Clang. The advantages I see here (admittedly, mostly for the implementation in Clang and LLVM):
It allows Clang and LLVM to model this with running an assembler over anything.
It doesn’t require new terminators in LLVM’s IR
We already have intrinsics in LLVM’s IR that could easily be extended to produce a nop.
It would be portable – each backend could select an appropriate sized nop to patch a jump into
I think inline assembly should work even if the compiler cannot parse the contents. This would rule out msvc inline assembly (or alternatively put all the parsing and interpretation burden on the frontend), but would work with gcc asm goto which specifies possible targets separately.
Supporting control flow in inline assembly by allowing jumps out of an assembly block seems natural to me.
Jumping into an inline assembly block seems like an unnecessary feature to me.
To have this working in lib/CodeGen we would need an alternative opcode with the terminator flag set. (There should also be opportunities to remodel some instruction flags in the backend, to be part of the MachineInstr instead of the opcode, but that is an orthogonal discussion to this)
I don’t foresee big problems in CodeGen, we should take a look on how computed goto is implementation to find ways to reference arbitrary basic blocks.
The register allocator fails when the terminator instruction also writes a register which is subsequently spilled (none of the existing targets does that, but you could specify this situation in inline assembly).
I’d always prefer intrinsics over inline assembly. Hey, why don’t we add a -Wassembly that warns on inline assembly usage and is enabled by default…
I still think inline assembly is valuable for new architecture bringup/experimentation situations.
I think inline assembly should work even if the compiler cannot parse the contents. This would rule out msvc inline assembly (or alternatively put all the parsing and interpretation burden on the frontend), but would work with gcc asm goto which specifies possible targets separately.
Supporting control flow in inline assembly by allowing jumps out of an assembly block seems natural to me.
Jumping into an inline assembly block seems like an unnecessary feature to me.
To have this working in lib/CodeGen we would need an alternative opcode with the terminator flag set. (There should also be opportunities to remodel some instruction flags in the backend, to be part of the MachineInstr instead of the opcode, but that is an orthogonal discussion to this)
I don’t foresee big problems in CodeGen, we should take a look on how computed goto is implementation to find ways to reference arbitrary basic blocks.
The register allocator fails when the terminator instruction also writes a register which is subsequently spilled (none of the existing targets does that, but you could specify this situation in inline assembly).
I’d always prefer intrinsics over inline assembly. Hey, why don’t we add a -Wassembly that warns on inline assembly usage and is enabled by default…
I still think inline assembly is valuable for new architecture bringup/experimentation situations.
To me, this feels like a great example of “we really wanted a language feature, but we figured out that we could hack it in using inline assembly in a way that’s ultimately significantly harder for the compiler to support than a language feature, and now it’s your problem.” I agree with Chandler that we should just design and implement the language feature.
I would recommend:
if (__builtin_patchable_branch(“section name”)) {
trace();
}
where @llvm.patchable_branch has the semantics of appending whatever patching information is necessary to the given section such that, if you apply the patch, it will change the result of the call from 0 to 1. That can then typically be pattern-matched in the backend to get the optimal codegen.
If I might recommend a better ABI for the patching information: consider using a pair of relative pointers, one from the patching information to the patchable instruction, and one from the patchable instruction to the new target. That would allow the patching information to be relocated at zero cost.
The actual details of how to apply the patch, and what the inline patchable-instruction sequence needs to be in order to accept the patch, would be target-specific. The documented motivating example seems to assume that a single nop is always big enough, which is pretty questionable.
This feature could be made potentially interesting to e.g. JIT authors by allowing the patching information to be embellished with additional information to identify the source branch.
- The register allocator fails when the terminator instruction also writes
a register which is subsequently spilled (none of the existing targets does
that, but you could specify this situation in inline assembly).
You can't actually have outputs from an asm goto in the GCC implementation
(and I'd suggest leaving that restriction in the LLVM implementation too if
it makes the implementation easier).
From GCC docs
<https://gcc.gnu.org/onlinedocs/gcc-6.3.0/gcc/Extended-Asm.html>:
"This form of asm is restricted to not have outputs. This is due to a
internal restriction in the compiler that control transfer instructions
cannot have outputs. This restriction on asm goto may be lifted in some
future version of the compiler. In the meantime, asm goto may include a
memory clobber, and so leave outputs in memory."
You can still have register clobbers, which I suppose might trigger the
same failure case?
I completely agree that for this example we rather want a proper intrinsic. As a matter of fact we have similar mechanism in CodeGen already to support the XRay feature.
I for one would really like that intrinsic. I have something similar under review, which wraps a function call for XRay’s custom event logging feature (I should’ve sent an RFC on this I realise, I’ll do that next). Patch doing this in particular for XRay’s requirements are in https://reviews.llvm.org/D27503 – wherein we do the following:
In LLVM IR, lower calls to the @llvm.xray.customevent(…) intrinsic into something like:
align to 2 byte address
.xray_sled_N
jmp +NN
calling convention setup
call <XRay’s trampoline>
We also mark the point where this sled is in the instrumentation map.
At runtime we overwrite the jump to become nops.
If we get this patchable branch intrinsic, then we can just certainly use that in lowering the XRay built-in we’re trying to add to Clang as well.
/me goes writing up the RFC for the custom event logging.
Let's not conflate the asm-goto part with the .pushsection/.popsection.
The latter ("0: .pushsection foo; .long 0b; .popsection") is used *all*
over the kernel to build up tables of code locations — for exception
handling of instructions which might fault, as well as for runtime
patching of instructions like the above. It's not always a nop vs. call
alternative.
It would be nice to have the compiler assist with that. We currently
have code to trawl through all the built object files and find calls to
__fentry__ so we can patch them in/out at runtime, for example. And we
might considered doing the same for calls to the retpoline thunks.
But I think we would be best served right now by considering that out
of scope, and looking *only* at the part which is handled by 'asm
goto'.
Collecting labels in code is a pretty interesting and reasonable use case.
I'd be really interested in a separate discussion or document describing
the kernel use cases for this. In particular, I'm worried about what things
LLVM might hoist across the label. We might want to end up designing
something more like real asynchronous exception handling (__try) than all
this assembly blob stuff.
But I agree, asm-goto is probably higher priority.
