Restrict input bits for analysis implementations in the presence of PC#659
Restrict input bits for analysis implementations in the presence of PC#659manasij7479 wants to merge 1 commit intogoogle:mainfrom
Conversation
|
I need more here. what's going on? why is it correct? |
|
add comments in the code explaining the strategy and rationale for correctness. |
|
remove commented-out lines of code unless they serve a specific purpose, in which case explain them |
|
ping |
|
Oh, forgot about this. |
manasij7479
force-pushed
the
input-restrictions
branch
from
32c9456 to
81a58a3
Compare
|
updated |
|
|
||
| std::unordered_map<Inst *, llvm::APInt> | ||
| PruningManager::computeInputRestrictions() { | ||
| std::unordered_map<Inst *, std::pair<llvm::APInt, llvm::APInt>> Seen; |
There was a problem hiding this comment.
you don't need to change it here but overall you should avoid using a std::pair for same-typed things like this, and instead pick an implementation which assigns appropriate names to things. here two separate variables would work, or else a struct with members called Zero and One or similar
|
I still have no idea why this does what you want it to do, you'll have to explain it more either in the code or in person |
|
@manasij7479 this is one of several things you need to get back to and finish up |
|
Addressed the comments, not sure what to do about the explanation. |
manasij7479
force-pushed
the
input-restrictions
branch
from
81a58a3 to
b22e861
Compare
|
how about some examples which show this working, then? |
|
I don't like random, complicated code which is neither tested nor well-explained. I know you can do better than this, please make an effort here. |
|
it seems pretty obvious what to do here:
|
|
I have no idea why you would be confident enough in this code's correctness to submit it for inclusion in souper without having done this |
2 participants
No description provided.