[PATCH] D32706: [AArch64] Consider lengthening instructions in cast cost calculation

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls added inline comments.


================
Comment at: lib/Target/AArch64/AArch64TargetTransformInfo.h:46-47
 
+  bool isLengthening(Type *Dst, Type *Src, const Instruction *I);
+
 public:
----------------
mssimpso wrote:
> kristof.beyls wrote:
> > I think the term "widening" is used for this operation, both in the LLVM code base, as well as in the ARMARM.
> > So, probably best to use isWidening here too.
> From what I've read, ARM uses "lengthening" to refer to the case where both operands are doublewords (e.g., uaddl) and "widening" to refer to the case when only one operand is a doubleword (e.g., uaddw). This patch only deals with the lengthening cases. We might handle the widening cases in the future, but they may not be as straightforward because the operand order matters. I can add a comment here explaining all of this.
I was making my comment based on Table C3-67 which contains UADDL in the ARMARM copy at https://static.docs.arm.com/ddi0487/b/DDI0487B_a_armv8_arm.pdf.
The table header is "SIMD widening and narrowing arithmetic instructions".
The table entry for UADDL has "UADDL, UADDL2 Unsigned add long (vector form)".
Therefore, I am assuming the L in the mnemonic stands for "long" not "lengthening".

Maybe there is other documentation that specifically calls this specific case "lengthening"?


================
Comment at: test/Analysis/CostModel/AArch64/lengthening-casts.ll:1-14
+; RUN: opt < %s -cost-model -analyze | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64--linux-gnu"
+
+; CHECK-LABEL: uaddl_8h
+; CHECK-NEXT:  Cost Model: Found an estimated cost of 0 for instruction: %tmp0 = zext <8 x i8> %a to <8 x i16>
----------------
mssimpso wrote:
> kristof.beyls wrote:
> > Next to tests that verify that a 0 cost is returned, maybe it'd also be useful to explicitly test that the widening instruction variant actually gets generated?
> We actually already have code generation tests for all of these cases over in test/CodeGen/AArch64 (see arm64-vadd.ll for the uaddl cases, for example). I just copied them over here to test the cost model. I'm fine adding an additional "llc" run-line to this test, though, that verifies the code generation property in this context. I checked and there are other cost model tests that do this (e.g., X86/scalarize.ll), so this wouldn't be that unusual.
Oh, we already have the tests to check the actual code generation. Than I think this is fine.
I think that you could argue both for and against the actual code generation test being in the same location as the cost model test.
I don't feel strongly enough either way to argue.


https://reviews.llvm.org/D32706