[PATCH] D76973: [LangRef] Clarify the semantics of branch on undef

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

I think the clarification allows to easily assume information from branch conditions without considering undef, but IIUC it makes introducing new branches harder? For example, the following would not be allowed, as it might introduce UB with the new definition, right? I mean that example below wouldn't work due to poison anyways, so I am not sure if it makes things actually worse in practice (there probably are other examples with values that possibly are undef rather than poison). But I think this kind of thing should definitely be highlighted somewhere as problematic (if my understanding is indeed correct). Introducing new branches out of 'thin' air is done in at least a couple of places (e.g. loop versioning, runtime checks and probably many more places).

  define i32 @src(i32 %x) {
    ret i32 %x
  }
  
  define i32 @tgt(i32 %x) {
    %c = icmp eq i32 %x, 10
    br i1 %c, label %true, label %false
  
  true:
    ret i32 10
  
  false:
  ret i32 %x
  }

As Eli pointed out in the discussion for D76611 <https://reviews.llvm.org/D76611>, there are quite a few places that introduce new branches which are already disabled for functions with the SanitizeMemory attribute, as this causes problems in practice already. I guess those all would need to be fixed and until all code is using the same assumptions, there will be bugs as we have seen already. Is there already an effort in that direction? If not, it might be good to at least record & track the issues. I would be happy to get something started in bugzilla or something else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76973