[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:170
+  %a0 = add i32 %x, %y
+  %m = mul i32 %x, 4
+  %a1 = add i32 %a0, %m
----------------
Allen wrote:
> spatel wrote:
> > 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. 
> ok, I'll delete the case shl_add_common_factor_commute2 with **shl** as it relies other transform.
> 
If the code patch is going to affect patterns with shl, then we must have tests that include shl. Do we need both mul and shl tests?


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

https://reviews.llvm.org/D136912