[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