fhahn added inline comments.

================
Comment at: clang/lib/Sema/SemaExpr.cpp:12112
+    return InvalidOperands(Loc, OriginalLHS, OriginalRHS);
+  }
+
----------------
rjmccall wrote:
> 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.
I am planning on adding support for compound assignment as follow-on patch and 
include the change there.


================
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)
----------------
rjmccall wrote:
> 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.
I've split up the tests more and updated the check lines to only check the bits 
relevant for the operator (loading operands, computation, storing result). The 
general matrix-type tests should cover checking loading the correct value. Was 
that what you had in mind?


================
Comment at: clang/test/SemaCXX/matrix-type-operators.cpp:188
+    // expected-note@-3 {{candidate function}}
+    // expected-note@-4 {{candidate function}}
+    return {};
----------------
rjmccall wrote:
> `expected-note` can take a repeat count
Ah that's great, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76793



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to