[PATCH] D99074: [llvm][AArch64][SVE] Fold literals into math instructions

[Joe Ellis via Phabricator via llvm-commits]

joechrisellis added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:1209-1219
+def fpimm_half : FPImmLeaf<fAny, [{
+  return Imm.isExactlyValue(+0.5);
+}]>;
+
+def fpimm_one : FPImmLeaf<fAny, [{
+  return Imm.isExactlyValue(+1.0);
+}]>;
----------------
nit: really small thing but the naming here is inconsistent with `fpimm0`. I would expect `fpimm1` and `fpimm2`. I suppose the difficulty here is shoehoring `fpimm_half` into this naming convention. If you can think of a sensible way of doing this, I would prefer we change this. Otherwise, ignore me. :-)


================
Comment at: llvm/test/CodeGen/AArch64/sve-fp-immediates-merging.ll:1
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+
----------------
General test suggestion -- prefer:

```
target triple = "aarch64-unknown-linux-gnu"

...

attributes #0 = { "target-features"="+sve" }
```

... and tag your functions with `#0`. I personally find this to be a bit more reliable when you're running `llc` on this file. :smile:

Also might be worth adding:

```
; RUN: ... 2>%t ...
; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
```


================
Comment at: llvm/test/CodeGen/AArch64/sve-fp-immediates-merging.ll:2
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+
+;
----------------
Please can we add:

```
; If this check fails please read test/CodeGen/AArch64/README for instructions on how to resolve it.
; WARN-NOT: warning
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99074