This is an archive of the discontinued LLVM Phabricator instance.

[ARM] [AARCH64] Add CodeGen IR tests for {VS}QRDML{AS}H v8.1a intrinsics.
ClosedPublic

Authored by labrinea on Dec 4 2015, 2:38 AM.

Diff Detail

Event Timeline

labrinea updated this revision to Diff 41854.Dec 4 2015, 2:38 AM
labrinea retitled this revision from to [ARM] [AARCH64] Add CodeGen IR tests for {VS}QRDML{AS}H v8.1a intrinsics..
labrinea updated this object.
labrinea updated this object.Dec 4 2015, 5:52 AM
echristo edited edge metadata.Dec 4 2015, 12:59 PM

Please remove the asm tests here. As I stated in the original review thread there's no reason for them to be here.

Thanks.

-eric

labrinea updated this revision to Diff 42035.Dec 7 2015, 2:00 AM
labrinea edited edge metadata.

ASM tests have been removed.

One inline comment, thanks!

-eric

test/CodeGen/aarch64-v8.1a-neon-intrinsics.c
4

Why do you need to enable the optimizers?

labrinea added inline comments.Dec 8 2015, 3:14 AM
test/CodeGen/aarch64-v8.1a-neon-intrinsics.c
4

Our intention with these tests is to check that we are generating a sequence of {v/s}qrdmulh, {v/s}q{add/sub}{s}, shufflevector, {insert/extract}element IR instructions. Using -O1 promotes memory to registers, combines instructions, and therefore decreases the context of IR that we need to check.

Should be pretty easy to either use CHECK-DAG or pick out the particular instructions you want to check here. Otherwise you're just checking how the optimizer runs. That, in particular, also sounds like a good backend check.

jmolloy resigned from this revision.Dec 12 2015, 5:33 AM
jmolloy removed a reviewer: jmolloy.

Eric is reviewing this; resigning myself.

Hi Eric,

The main optimization I feel is useful is mem2reg. Without that, if I want to properly check the right values go to the right operands of the intrinsic calls I have to write FileCheck matchers that match stores and their relevant loads, plus bitcasts. This not only looks more obfuscated than matching the mem2reg output, but it is also less resilient to changes in the way clang code generates.

The generated IR for each intrinsic is around 50 lines. I can just pick out the particular instructions I want to check, as you suggested, but they won't we connected by the flow of values. In my opinion such a test will be less valuable.

I can do this both ways but my preferred way is to run the bare minimum of optimization to de-cruft the output and make the test robust and readable. If you feel however that you don't want the optimizers run I will make a best effort at writing a test that doesn't use them.

I understand the conflicting priorities here for sure. You'd like a test that's as minimal as possible, without having to depend on external (to clang) libraries here. I really would appreciate it if you'd make the test not rely on mem2reg etc so we can be sure that clang's code generation is the thing tested here and not the optimizer. Making sure that the unoptimized output reduces properly would be a great opt test for the backend though.

Thanks!

-eric

labrinea updated this revision to Diff 43885.Jan 4 2016, 6:37 AM
labrinea updated this object.

Disabled optimizers.

echristo accepted this revision.Jan 4 2016, 3:01 PM
echristo edited edge metadata.

LGTM, and thanks for all of the iteration.

-eric

This revision is now accepted and ready to land.Jan 4 2016, 3:01 PM
This revision was automatically updated to reflect the committed changes.