-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
[simplifycfg] constantexpr divide evaluation made unconditional #1329
Comments
assigned to @lattner |
To reproduce: opt -predsimplify pr957.bc > bug.bc Look for cond_true182 |
I can't reproduce this. When I run -predsimplify over your testcase I get the |
Try the new test case. Take a look at BB cond_true182. |
Predsimplify is right. Here's the input code: cond_false179: ; preds = %cond_true cond_true182: ; preds = %cond_false179 cond_next185: ; preds = %cond_true182, %cond_false179 predsimplify decides to hoist the def'n of %tmp184 into cond_next185. If %tmp is cond_false179: ; preds = %cond_true cond_true182: ; preds = %cond_false179 cond_next185: ; preds = %cond_true182, %cond_false179 The "div (uint 1, uint 0)" should only happen if the flow came from |
Nick is right. Consider this testcase: uint %test(uint %tmp) { cond_true182: ; preds = %cond_false179 cond_next185: ; preds = %cond_true182, %cond_false179
} gccas turns this into: uint %test(uint %tmp) { which unconditionally executes the div. This is a bug in something in gccas (probably instcombine or simplifycfg), predsimplify just exposes it. -Chris |
simplifycfg does this. |
Why does a select instruction unconditionally evaluate both arguments? Shouldn't If I had to follow this rule as implied, then there is a bug in predsimplify, |
select is closest to a conditional move. Given two values (which are precomputed) it picks one. Select Fixed. Patch here: testcase here: SimplifyCFG/2006-10-19-UncondDiv.ll -Chris |
That's not good. That means that I can't use |
Yes you can, why wouldn't you be able to? |
Actually, I take it back. I can't see any way in which this choice of semantics If there is a bug, the fix doesn't involve iterating. It's just a matter of File a new bug if you can manage to demonstrate the equivalent bug in predsimplify. |
yep |
Extended Description
Without predsimplify pass:
LBB1_18: #cond_false179
testl %ebp, %ebp
jne LBB1_20 #cond_next185
LBB1_19: #cond_true182
movl $1, %eax
xorl %edx, %edx
divl %ebp
movl %eax, %ebp
LBB1_20: #cond_next185
cmpl $65535, %ebp
ja LBB1_24 #cond_false199
The divl is properly guarded to prevent divide by zero.
With -predsimplify:
LBB1_25: #cond_true182
xorl %edx, %edx
movl $1, %eax
divl %edx
movl %eax, %ebp
LBB1_26: #cond_next185
cmpl $65535, %ebp
ja LBB1_30 #cond_false199
That's guaranteed to cause an divide by zero exception!
The text was updated successfully, but these errors were encountered: