-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make better use of conditional unreachable's in LLVM #1182
Comments
Basic question about "ASSERT": why bother? LLVM already has the ability to express "if this condition isn't Assume is useful as an intrinsic, and could conceptually be useful for conveying information from the Why do you need assume for loop invariants? Those are trivial to compute. -Chris |
The addvantage of having explicit ASSERT statement is that it can be used by Here's an example: let's assume that you're compiling some kernel (linux/or As about loop invariants, I disagree. Can you suggest an algorithm to find an I can't, and I'll happily pay a beer to anyone who can. Additional predicates can be introduced when and if needed. Currently there's Furthermore, LLVM is lacking a language-independent attribute system. Assumes And, I wonder whether that statement about code blowup is true. How many |
I still don't understand your comments about assert. Any transformation tool that can insert an assert I don't understand your comment about the loop invariant. There are no loop invariants in that loop, at
Code blowup isn't a matter of how many unique predicates you need, it's a question of how many
This is a feature, not a bug. :) -Chris |
Yes, and everyone can use GCC as a front-end. :-) The fact that something can be done doesn't mean that it should be done
There is an invariant, y1=GCD(x1,x2) && y3+y4=LCM(x1,x2). The point is
The code can be annotated with assumes/asserts but doesn't have to be.
Right, not a bug, but a lacking feature. Domagoj |
I have no idea what this refers to.
Agreed, what's your point?
Please please please start using standard terminology. LLVM already has a mechanism for doing this,
You are conflating the ideas of annotations in the source code (which I don't care about) with the size of
No it isn't. Please see the extensive discussions in the llvmdev mailing list on this topic. -Chris |
The point is that having an ASSERT intrinsic is a better way to handle
I'm using standard terminology. You're missing the point. Pass
Can you propose an alternative way to get that information into LLVM BTW, you would have at most the number of predicates that is equal to
Which topic exactly? Domagoj |
While I understand that this is your opinion, you haven't said anything that backs that up. Real code
Not true. This doesn't have pass ordering problems because it's not powerful enough to express such Reading/writing pass results into .bc files would have exactly the same expressiveness as bolting on
Extend the IR.
You're missing that this is inherently a language extension that you're talking about here. Why not
Incorrect. First, programmers annotate non-ssa variables. Once in SSA form, one non-ssa variable can Further, to illustrate that this is a language extension, LLVM transformations need to know what these
If you look, you'll find it. I would find it no quicker than you could. It has been discussed extensively -Chris |
I just don't see how handling an arbitrary amount of code and figuring
Being simple is a feature, not a bug, that's why asserts/assumes are
Look, I don't really care that much how assertions are supported.
All true, but I simply don't see those millions of intrinsics.
Let me rephrase:
What are you talking about? What should I look-up? Discussions on Domagoj |
A simpler way to do this: |
Would it not be more compact to use an undef label? That is: br i1 %condition, label %labelIfTrue, label undef rather than br i1 %condition, label %labelIfTrue, label %labelIfFalse |
Assigning this to myself, in the absence of anyone else looking at it. This kind of functionality blocks a few possible optimizations in code that Currently this functionality is blocked by the SimplifyCFG pass that runs early Nick Lewycky apparently experimented with solving this bug, leaving unreachable |
Here's a few details that haven't been captured in the bug but were discussed on the mailing list, etc: Mailing list (2/2/2010) IRC (10/30/2011) From the last email in the thread, Jakob mentioned: "I would like this patch better if you would implement Chris' suggestion of DCE'ing unreachable blocks with conditions that are too gross for the optimizer. As it stands there can be arbitrarily complex instructions feeding into the condition." Though I can't find any mention of the comment by Chris that Jakob is alluding to. On the one hand I can see the merit in only working with sufficiently simple expressions & killing anything too complicated up-front/early, I sort of really want to see what code would be like if we could benefit from arbitrary 'assertions' through this mechanism. I'm just not sure it'll be possible without hurting other optimizations. I'll try it out by updating Nick's patch & adding his suggestion of using InsntSimplify after removing unreachables as a way to see if that gets the code back on track. I'll update this bug with an updated patch & any progress/details. |
This old bug seems to cover both the original rationale for creating 'unreachable' and more recent efforts to let LLVM make better use of 'unreachable' for optimization purposes. The following simple example demonstrates the issue. void f1(); void f2(); void abort() attribute((noreturn)); void test1(int x) { void test2(int x) { |
I think this has been addressed in general. For the example code mentioned in the previous comment is optimised as expected I think: https://godbolt.org/z/39cdJa for test1 we generate a single comparison, and test2 is optimised to just call f2(). Given that, I think we can close this issue. Apologies if I missed something from the extensive discussion in the issue. |
It seems that the current trunk version of clang (as of 2020-09-02) does not optimize away the call to f1 in test2 (nb. clang 9 and 10 did optimize it). See: https://godbolt.org/z/YK13j1. |
That is unfortunate! Given that it is a recent regression, it is probably better to create a new issue, as it will require to track down where/when we regressed again. |
Extended Description
Hi,
I'd like to open a discussion on adding assert/assume intrinsics to
LLVM. Here I will describe the syntax, semantics, and applications
of both.
Those statements should have the following syntax:
assert/assume ( Boolean_variable, String )
"Boolean_variable" represents a condition.
The only purpose of "String" is to classify assertions and assumptions
into classes, so that, for example, certain class of assertions can be
enabled or disabled during the code generation. For maximal generality,
I'd suggest leaving "String" unspecified for now. The users can come up
with identifiers like "Range_check" or similar.
The semantics of statements is:
ASSERT - if the condition evaluates to TRUE, the statement is a NOP,
otherwise the assertion terminates the program. Code generators should
be able to enable/disable assertions, but are also allowed to completely
ignore them.
ASSUME - is always a NOP, but the code generators might generate code
that issues a warning if the condition evaluates to FALSE. Assumptions
present the conditions that "should", but are not guaranteed to hold.
Applications:
ASSERT - debugging, code reliability, can represent both user provided
assertions (invariants, pre/post-conditions) and automatically inserted
checks, like range checking.
ASSUME - primarily as a tool for communication between passes. Some
possible applications include:
language independent attribute mechanism
[ like assume(restricted(ptr_x), "Restricted Pointer") ]
passing information through different stages of the compilation
framework, for example, a loop analysis pass might store an inferred
loop invariant as assume(x + y < 200, "Loop Invariant").
static checking. For details I'd suggest papers from Dijkstra, Greg
Nelson, Rustan Leino.
It should be noted that ASSUMEs have application specific meaning, and
should not be touched by passes that do not use them (something like dbg
intrinsics).
Looking forward to your comments,
Domagoj Babic
The text was updated successfully, but these errors were encountered: