On 10/18/13 12:47, Marc Glisse wrote:
Maybe a new -fretroactive-undefined-behavior? (for later, obviously)
Something like that -- haven't thought much about the name.
* should cfg_altered be static (or a member of the pass class)?
At the minimum it should be static. Into the pass class would be
better. I'll take care of that.
* tree-vrp has a function infer_nonnull_range, do you think we could
share it? We now store the VRP ranges for integers, but not for
pointers. If we did (or maybe just a non-null bit), the pass could just
test that bit on the variable found in the PHI.
I'm not sure what can really be shared here -- this patch searches for
PHIs where one or more of the args is a NULL pointer. The NULL pointer
will be explicit in the PHI arg. So a real world example (alias.i from
gcc-4.6.0):
<bb 67>:
# iftmp.163_8 = PHI <iftmp.163_111(66), 0B(65), 0B(93)>
# _241 = PHI <_242(66), _109(65), _246(93)>
_112 = iftmp.163_8->u.hwint[0];
_113 = _241 + _112;
_114 = _113 * 8;
_115 = ref_28(D)->size;
if (_114 == _115)
goto <bb 87>;
else
goto <bb 68>;
So when BB67 is reached via BB65 or BB93 iftmp.163_8 will have the value
0 via the PHI node at the start of BB67 and it'll be derefereced in
BB67. We'll isolate the 65->67 path and the 93->67 path.
What you're suggesting would be more useful for a warning pass which
detected potential null pointer dereferences. If we continue with the
same example and assume we isolated the two paths noted above we have
this remaining as BB67:
<bb 67>:
# iftmp.163_8 = PHI <iftmp.163_111(66)>
# _241 = PHI <_242(66)>
_112 = iftmp.163_8->u.hwint[0];
_113 = _241 + _112;
_114 = _113 * 8;
_115 = ref_28(D)->size;
if (_114 == _115)
goto <bb 87>;
else
goto <bb 68>;
If we've determined via VRP that iftmp.163_111 has a non-null property,
then we would not warn about a potential null pointer dereference.
That's an example of where preserving the range information for pointers
out of VRP would be useful.
I actually have another patch which does precisely these kinds of
warnings. It needs more TLC before we could even consider integrating it.
BTW, if we continue with that example, immediately after isolation we
have (I'm including our block's predecessor because it's going to
merge). BB63 is the pred, BB64 is the block where we isolated out two
paths:
;; basic block 63, loop depth 0, count 0, freq 529, maybe hot
;; prev block 62, next block 64, flags: (NEW, REACHABLE)
;; pred: 62 [27.0%] (TRUE_VALUE,EXECUTABLE)
iftmp.161_102 = _99->size;
_186 = iftmp.161_102->u.hwint[0];
_69 = mem_25(D)->u.fld[1].rt_mem;
iftmp.163_111 = _69->offset;
;; succ: 64 [100.0%] (FALLTHRU,EXECUTABLE)
;; basic block 64, loop depth 0, count 0, freq 623, maybe hot
;; prev block 63, next block 65, flags: (NEW, REACHABLE)
;; pred: 63 [100.0%] (FALLTHRU,EXECUTABLE)
# iftmp.163_8 = PHI <iftmp.163_111(63)>
# _241 = PHI <_186(63)>
_112 = iftmp.163_8->u.hwint[0];
_113 = _241 + _112;
_114 = _113 * 8;
_115 = ref_28(D)->size;
if (_114 == _115)
goto <bb 83>;
else
goto <bb 65>;
We soon realize that we can cprop away the two PHI nodes at the start of
BB64 and merge BB64 and BB63 together resulting in:
;; basic block 63, loop depth 0, count 0, freq 529, maybe hot
;; prev block 62, next block 64, flags: (NEW, REACHABLE)
;; pred: 62 [27.0%] (TRUE_VALUE,EXECUTABLE)
iftmp.161_102 = _99->size;
_186 = iftmp.161_102->u.hwint[0];
_69 = mem_25(D)->u.fld[1].rt_mem;
iftmp.163_111 = _69->offset;
_112 = iftmp.163_111->u.hwint[0];
_113 = _112 + _186;
_114 = _113 * 8;
_115 = ref_28(D)->size;
if (_114 == _115)
goto <bb 81>;
else
goto <bb 64>;
Later we discover some CSE opportunities and remove dead code resulting in:
;; basic block 66, loop depth 0, count 0, freq 529, maybe hot
;; prev block 65, next block 67, flags: (NEW, REACHABLE)
;; pred: 65 [27.0%] (TRUE_VALUE,EXECUTABLE)
_186 = _91->u.hwint[0];
_113 = _166 + _186;
_114 = _113 * 8;
_115 = ref_28(D)->size;
if (_114 == _115)
goto <bb 67>;
else
goto <bb 68>;
Needless to say, this is goodness.
Jeff
