I donāt want to touch __builtin_assume with the flag, if someone wants to disable that, it should perhaps be a separate flag. As far as the above pattern: Iām kind of the mind of, ādisabling this is effectively getting you back to pre-attribute behavior, so if the programmer REALLY wanted to assume, the feature-test-macro check is their way out (backing up to the builtin)ā.
I think the ādefaultā should be the result of the #1 vs #3 vote above, and it looks like #3 is winning, so Iād expect āassumeā to be default.
Thank you everyone for the good discussion around this RFC! Now that discussion seems to have wound down, I think weāve got consensus on a way forward.
Do not change the behavior of __builtin_assume
Support [[assume]] by lowering its operand to LLVM IR
Add an on/off driver flag allowing the user to enable or disable the [[assume]] attribute as they need
Enabled by default in C++23 mode
The rest of the details can be hammered out during review regarding the naming of the driver flags, whether we want to support the attribute in older standards modes or C, etc.
It would also be good to just add checking of the assumption to ubsan (but only if it has no side effects). It definitely fits the kinds of things ubsan is designed to catch.
Yeah, I donāt think it should block the initial commits. Just wanted to make sure that was consensus on the resolution to how to check them and didnāt see it in the list, but if that was just for the initial work then thatās fine.
Yep, rather than the discussion about whether enforce is a dialect/etc - +1 to enforcement via ubsan being consistent with other undefined behavior checking.
(has anyone written up more detail on what assumptions tend to risk negative perf consequences? Iād have thought it was basically anything non-transparent to the compiler? Like we donāt have any way to mark the contents of the assumption has side-effect-agnostic, right? So if you just put an arbitrary function call in your assumption, LLVM doesnāt know it can drop it, so preserves it etc - even some fairly broad description like that seems plausible to document, again, for the kinds of power users reaching for this sort of tool anyway - leaving folks hopefully adding more direct assumptions like āthis thing is non-null, non-zero, less than 47, etcā which Iād expect to be pretty benign/helpful.
Could even go so far as to warn if an external function call is made from an assumption - but thatād only catch the superficial cases, not the case of calling an inline function that then calls an external function, etcā¦ )
Hmm, maybe Iām misunderstanding something here, but if the assumption has side-effects at the C++ level, then we currently just donāt emit anything at all for it in Clang codegen (and issue a warning that the expression has side-effects) for __builtin_assume (and also [[assume]] in the current implementation).
Ah, so it does - even an inline function call gets the warning and is discarded/not emitted into the LLVM IR.
I was just making a rough guess - hadnāt checked the implementation in a while - so not sure what might be the common idioms that lead to inefficiencies in code generation due to assumptions at this pointā¦
I was wondering if improvement to the llvm assume intrinsic could potentially make for a good GSoC project as it seems that no one has time to work on that otherwise?
Would you be amenable to updating The LLVM Compiler Infrastructure Project and moving it up to the 2024 set of open projects weāre considering? Maybe that will help entice a student to apply?
This would be wonderful. Iād be happy to help in any way I can (Sadly, anything past the clang front-end could as well be black magic to me so I probably would not be able to help much)
