[PATCH] D80552: [PrintSCC] Fix printing a basic-block without a name

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 29 08:40:05 PDT 2020


fhahn added inline comments.


================
Comment at: llvm/test/Other/print-cfg-sccs.ll:20
+2:
+  br i1 undef, label %1, label %3
+
----------------
ekatz wrote:
> baziotis wrote:
> > ekatz wrote:
> > > fhahn wrote:
> > > > nit: branch on undef is UB. It would probably be better to pass in a condition as argument or something like that.
> > > You are correct of course, but I have seen many other test cases that take the `undef` approach where all that matters is the CFG, and not the contents. for example "loop-pass-ordering.ll" (in the same directory).
> > I'm not familiar with a big part of LLVM test-suite, but any test cases that branch on `undef` (or generally, that they have some `undef` or whatever UB) it was intended for specific reasons (for example, to test UB recognition or it was propagated somehow).
> > I also think that it would be better to use a valid value. Otherwise it looks good :)
> As far as I know, it is ok to use `undef` in a brach if the destination doesn't matter.
> Anyway, I think there is no need to argue, as there is a very easy fix for it - I'll just add a function argument for the condition.
> As far as I know, it is ok to use undef in a brach if the destination doesn't matter.

IIRC some passes had/have this interpretation, but it was recently clarified in the LangRef to make branch on undef UB explicitly (rG05f0e598ab265a80fedb23225cde4176f11774ac)

> Anyway, I think there is no need to argue, as there is a very easy fix for it - I'll just add a function argument for the condition.

Thanks!


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

https://reviews.llvm.org/D80552





More information about the llvm-commits mailing list