[PATCH] D79712: [AArch64][BFloat] add BFloat instruction support for AArch64

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli requested changes to this revision.
fpetrogalli added a comment.
This revision now requires changes to proceed.

HI @stuij ,

thank you for working on this. Please make sure that you use check-label and not just check when looking for the symbols of the functions. I have marked one place where this is happening. Also, please remove the tests that are checking the "kill" comments (one marked in the tests). Please make sure you fix it in all test cases before submitting.

Feel free to address also the two nits below if they make sense to you, but I won't hold my position on those. :)

Francesco

Nit1: I think you should make sure that nothing else than the instruction you are testing is generated, by invoking `llc` with `-asm-verbose=0` and modifying the tests as follows:

  define <4 x i16> @v4bf16_to_v4i16(float, <4 x bfloat> %a) nounwind {
  ; CHECK-LABEL: v4bf16_to_v4i16:
  ; CHECK-NEXT: mov v0.16b, v1.16b
  ; CHECK-NEXT: ret
  entry:
    %1 = bitcast <4 x bfloat> %a to <4 x i16>
    ret <4 x i16> %1
  }

Nit2: You mark some functions with `#0`, which is not defined anywhere, please remove it.



================
Comment at: llvm/test/CodeGen/AArch64/bf16-vector-shuffle.ll:19
+; CHECK:       // %bb.0: // %entry
+; CHECK-NEXT:    // kill: def $h0 killed $h0 def $q0
+; CHECK-NEXT:    dup v0.4h, v0.h[0]
----------------
What are you testing here? These lines seem not necessary in terms of code generation. Please remove, here and in all other tests.


================
Comment at: llvm/test/CodeGen/AArch64/bf16.ll:7
+define bfloat @test_load(bfloat* %p) {
+; CHECK: test_load
+; CHECK: ldr h0, [x0]
----------------
Please use check-label, not check, when testing symbols in this file. Also, please make sure you test the end of the function name by testing also the final `:`, as in `// CHECK-LABEL: test_label:`

Please fix all the tests in this file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79712