[PATCH] D133287: [InstCombine] Test for matrix multiplication negation optimisation.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/matrix-multiplication-negation.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --force-update
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
----------------
nit: is `UTC_ARGS: --force-update` really needed?


================
Comment at: llvm/test/Transforms/InstCombine/matrix-multiplication-negation.ll:4
+
+define <2 x double> @test_negation_move_to_result(<6 x double> %a, <3 x double> %b) {
+; CHECK-LABEL: @test_negation_move_to_result(
----------------
could you also add versions of `test_negation_move_to_result` & `test_negation_not_moved` where the second operand is negated instead?


================
Comment at: llvm/test/Transforms/InstCombine/matrix-multiplication-negation.ll:15
+
+define <9 x double> @test_move_negation_to_second_operand(<27 x double> %a, <3 x double> %b) {
+; CHECK-LABEL: @test_move_negation_to_second_operand(
----------------
Could also add another version of the test where the negation is moved from the second to the first operand?


================
Comment at: llvm/test/Transforms/InstCombine/matrix-multiplication-negation.ll:26
+
+define <15 x double> @test_negation_not_moved(<3 x double> %a, <5 x double> %b) {
+; CHECK-LABEL: @test_negation_not_moved(
----------------
Could you add quick comments to the tests saying why the negation should or should not be moved?


================
Comment at: llvm/test/Transforms/InstCombine/matrix-multiplication-negation.ll:68
+  %a.neg = fneg <27 x double> %a
+  %b.neg = fneg <3 x double> %b
+  %res = tail call <9 x double> @llvm.matrix.multiply.v9f64.v27f64.v3f64(<27 x double> %a.neg, <3 x double> %b, i32 9, i32 3, i32 1)
----------------
`%b.neg` isn't used?


================
Comment at: llvm/test/Transforms/InstCombine/matrix-multiplication-negation.ll:89
+
+define <15 x double> @chain_of_matrix_mutliplies(<3 x double> %a, <5 x double> %b, <35 x double> %c) {
+; CHECK-LABEL: @chain_of_matrix_mutliplies(
----------------
AFAICT there's no case at the moment where it would be profitable to move the negation with chains, right? Could you add it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133287