[PATCH] D50659: [CodeGen] Fix remaining zext() assertions in SelectionDAG
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 31 11:17:56 PDT 2018
efriedma added inline comments.
================
Comment at: unittests/CodeGen/SelectionDAGTest.cpp:39
+ if (!T)
+ return;
+
----------------
scott.linder wrote:
> efriedma wrote:
> > This seems bad; the test silently fails to run if the AArch64 target isn't built?
> This does seem bad, but as far as I can tell there is no appealing alternative. Not that it absolves me of this, but other tests (e.g. CodeGen/GlobalISel/LegalizerHelperTest.h) do the same. The core issue is not having a "generic target" always available for unittests.
>
> Is there anything better to do in this case?
How hard would it be to make a fake target for SelectionDAG unittests? A skeleton implementation of a Target, a TargetMachine, and a TargetSubtargetInfo is hopefully not that much code. But maybe it's not quite that simple...
That said, I'm not really that worried about the part where the test only runs if the aarch64 target is enabled; we have target-specific tests all over. Maybe enough to just rename SelectionDAGTest to AArch64SelectionDAGTest so someone doesn't accidentally add a new test without running it.
Repository:
rL LLVM
https://reviews.llvm.org/D50659
More information about the llvm-commits
mailing list