[PATCH] [SimplifyCFG] Double check for constant expression	domination
    Sergey Dmitrouk 
    sdmitrouk at accesssoftek.com
       
    Fri Oct  3 10:09:12 PDT 2014
    
    
  
Sorry for the delay.
> This does not seem like the right fix. What is isSafeToSpeculativelyExecute checking that C->canTrap is not checking?
Looks like I misread the code of `canTrap`.
> I'd think that fixing C->canTrap is the right way to address this problem.
It's not broken at the moment. It didn't work the way I'd expect after I modified `isSafeToSpeculativelyExecute`. If I get it right this time, I need to duplicate checks in `isSafeToSpeculativelyExecute` and `canTrap`, because the former one doesn't call the later one.
> I'd also like to make a plea for a testcase here. If we had a testcase, 
> we could see for ourselves what isSafeToSpeculativelyExecute is checking 
> that c->canTrap isn't.
It works fine while these two functions are in sync, I missed that it's important to update both of them at the same time.
Thanks, now I understand that new checks should be introduced in a different way.
http://reviews.llvm.org/D5459
    
    
More information about the llvm-commits
mailing list