[PATCH] D154307: [NewGVN] Add test with assume dominating an ICmp. NFC

[Konstantina Mitropoulou via Phabricator via llvm-commits]

kmitropoulou added a comment.

@ManuelJBrito You are right :) I missed this. NewGVN checks if it can simplify icmp eq i32 %xor, 0. For this reason, it calls simplifyICmpInst() from InstructionSimplify.cpp. This function tries to find if the compare is dependent on a llvm.assume() intrinsic. Hence, it calls isValidAssumeForContext() which returns true for both cases! The isValidAssumeForContext() checks if the llvm.assume() dominates the icmp eq i32 %xor, 0. The problem is that  icmp eq i32 %xor, 0 does not have a parent. Because of that, dominates() get confused that icmp eq i32 %xor, 0 is part of an unreachable block and it returns true.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154307/new/

https://reviews.llvm.org/D154307