rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land.
LGTM. ================ Comment at: clang/lib/Sema/SemaExpr.cpp:12112 + return InvalidOperands(Loc, OriginalLHS, OriginalRHS); + } + ---------------- fhahn wrote: > 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. WFM ================ 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) ---------------- fhahn wrote: > 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? Yes, this looks 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
