[PATCH] D136912: [AArch64] precommit tests and code format for D136623, NFC

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/add4.ll:169
+;
+  %a0 = add i32 %x, %y
+  %m = mul i32 %x, 4
----------------
This isn't a different commuted pattern than above. Did you want to commute operands for %a0?


================
Comment at: llvm/test/Transforms/InstCombine/add4.ll:170
+  %a0 = add i32 %x, %y
+  %m = mul i32 %x, 4
+  %a1 = add i32 %a0, %m
----------------
I don't understand the motivation to include tests that will be converted to something else (`shl`) when we also have an explicit test for the `shl` patterns below here. If we make this `mul` with some arbitrary constant (for example, 42), then it is clear that we are testing the various commuted pattern possibilities for a real `mul` (there should be 4 of those?). 

If you want to add more tests to verify that `shl` transforms are working, then write the IR and the test comments in that form to begin, so we are not relying on some other transform to produce the pattern that is tested. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136912/new/

https://reviews.llvm.org/D136912