ASDenysPetrov reopened this revision.
ASDenysPetrov added a comment.
This revision is now accepted and ready to land.
I've just investigated this patch deeply. I think we have wrong logic here.
Assume we have `(int)(short)(int x)`. `VisitSymbolCast` will try to get the
constant recursively in the next order:
- `(short)(int x)`
- `(int x)`
And here is my concern. Whether it is correct to get the range for `int x` and
consider it as a correct simplification of `(int)(short)(int x)`. IMO it can't
be simplified like that. It should go through the set of conventions and
intersections.
Working on an integral cast feature I've encountered in the situation when I
can get the range for `(short)(int x)` in the chain above. And it logically
matches with the original symbol `(int)(short)(int x)`. After that, your
solution (this patch) stops and returns this range, missing the range
associated with `int x`. That borns the problem:
| **before my patch** | **after my patch**
|
| (int)(short)(int x) = 0 | (int)(short)(int x) = 0
|
| (short)(int x) = ? / go next | (short)(int x) = 0 / match
|
| (int x) = 1 / match | (int x) = 1 / doesn't reach because of the
previous match |
| **result** | **result**
|
| 0 and 1 infeasible | 0 and 0 feasible
|
|
I think we have to improve this behavior.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126481/new/
https://reviews.llvm.org/D126481
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
