[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