fhahn marked 4 inline comments as done.
fhahn added inline comments.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:15051
+
+ Expr *Arg = TheCall->getArg(0);
+ if (!Arg->getType()->isConstantMatrixType()) {
----------------
rjmccall wrote:
> fhahn wrote:
> > rjmccall wrote:
> > > When a builtin has custom type-checking (`t`), you need to handle
> > > placeholders in the operands yourself, just like you would for an
> > > operator.
> > I added placeholder handling and added additional tests.
> Oh, I'm sorry, I gave you poor advice: you do need to handle placeholders,
> but more generally than that, you need an r-value. Since you aren't doing
> any further conversions, you just need to call DefaultLValueConversion, which
> will also handle placeholders for you.
>
> You will also need to store the checked argument back into the call, which
> you can only really test with an IRGen test. This is one of the few places
> where we do actually directly mutate the AST.
Oh right, I wasn't sure about whether we need DefaultLValueConversion here or
not. I tried to construct a C/C++ test case that would actually require it, but
couldn't come up with a test. In any case, I updated it to use
`DefaultLvalueConversion` instead and also adjust the argument before returning
the updated call.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:1918
+ switch (BuiltinID) {
+ case Builtin::BI__builtin_matrix_transpose:
+ return SemaBuiltinMatrixTransposeOverload(TheCall, TheCallResult);
----------------
rjmccall wrote:
> I didn't notice this before, but I think a single level of switch is fine;
> there's probably nothing common about matrix builtins that you're going to
> want to handle like this.
It's not required any more, after removing the `fenable_matrix` check. I
dropped it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72778/new/
https://reviews.llvm.org/D72778
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
