[PATCH] D76793: [Matrix] Implement + and - operators for MatrixType.
John McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu May 28 10:59:06 PDT 2020
rjmccall added a comment.
Thanks, functionality is looking good for the non-assignment operators.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:12112
+ return InvalidOperands(Loc, OriginalLHS, OriginalRHS);
+ }
+
----------------
You need to not actually apply this conversion to the LHS if this is a compound assignment. You can handle that in a follow-up patch, I think, since this patch isn't doing those yet.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:8591
+ /// Add binary operators overloads for each candidate matrix type M1, M2:
+ /// * (M1, M1) -> M1
----------------
"operator overloads"
================
Comment at: clang/test/CodeGenCXX/matrix-type-operators.cpp:374
+ // CHECK-NEXT: %1 = bitcast [90 x double]* %value to <90 x double>*
+ // CHECK-NEXT: %2 = load <90 x double>, <90 x double>* %1, align 8
+ // CHECK-NEXT: %call = call double @_ZN14DoubleWrapper1cvdEv(%struct.DoubleWrapper1* %w1)
----------------
Please don't test for exact value numbers; it makes updating the test really painful for relatively minor IR changes. Use FileCheck variables instead.
More generally, can you make these tests a little more targeted instead of testing the entire function body? You might find it easier to do so if you do a single operation per function.
================
Comment at: clang/test/SemaCXX/matrix-type-operators.cpp:188
+ // expected-note at -3 {{candidate function}}
+ // expected-note at -4 {{candidate function}}
+ return {};
----------------
`expected-note` can take a repeat count
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76793/new/
https://reviews.llvm.org/D76793
More information about the cfe-commits
mailing list