[PATCH] D46072: [DagCombine][InstCombine][NFC] De Morgan law tests

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D46072#1080278, @spatel wrote:

> > I think for DAGCombine we should be testing MIR output.
>
> That's more limited & stable, but there's a tradeoff. It means that we're requiring reviewers/contributors to understand another syntax/mini-language in addition to the IR input and their favorite target's asm output. I think that's why, in x86 at least, we still mostly have tests output to asm. Also with small & focused tests, the likelihood of something later in the pipeline altering the expected output is low, so we're fine testing x86 asm as the output.


One thing to note - i specifically want to see the post-dagcombine output, and do not want the latter passes to interfere, even potentially, thus it is outputted as MIR.
That being said, the aarch64 mir output already looks somewhat alien, so yes, maybe i could test the final asm, for the most dumbest target with no special intrinsic sets.



================
Comment at: test/CodeGen/AArch64/demorgan-extra.ll:4-6
+; There is a identical twin test in test/Transforms/InstCombine/demorgan-extra.ll
+; Please keep them in sync! :)
+
----------------
spatel wrote:
> I understand that we want to coordinate instcombine and DAGCombine here, but I don't agree with this requirement. In particular, it doesn't make sense to have a big chunk of tests with illegal types in the DAG. (It doesn't add much to instcombine at this point either - I'm confident that APInt works as advertised, and we have plenty of tests for weird types already.)
> 
> So several tests here don't add enough value to justify their cost IMO (similar comment as in D46008) because we try *not* to produce those types in the first place. It's nice to have a bit of coverage to see that we don't miscompile, but that's about it.
> 
> So again, this is just my opinion, but I'd rather see more economical use of tests and less tests overall. I do appreciate the thoroughness and test comments though. :)
I think you meant to post that on `test/CodeGen/*/demorgan.ll`, i'd be ok with dropping them from dagcombine.
But not these `demorgan-extra.ll` tests. I need them.


Repository:
  rL LLVM

https://reviews.llvm.org/D46072