[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

arsenm closed this revision. arsenm added a comment. 2ad4c3c88d884684a3efb42181e87fe305df51bd CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140639/new/ https://reviews.llvm.org/D140639 __

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140639/new/ https://reviews.llvm.org/D140639 ___ cfe-commits mailing lis

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

arsenm added a comment. Herald added a subscriber: StephenFan. ping, I think this should get in before the branch date to fix the current broken behavior before this is in a release CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140639/new/ https://reviews.llvm.org/D140639

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

arsenm updated this revision to Diff 486643. arsenm added a comment. Return S.Diag CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140639/new/ https://reviews.llvm.org/D140639 Files: clang/lib/Sema/SemaChecking.cpp clang/test/CodeGen/builtins-elementwise-math.c clang/test/Sema/buil

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

erichkeane added a comment. I still want someone more familiar with LLVM to review this, but code wise I think we're ok (modulus 1 suggestion I made in checkFPMathBuiltinElementType). Comment at: clang/lib/Sema/SemaChecking.cpp:2674 + +if (MagnitudeTy.getCanonicalType() !=

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:2674 + +if (MagnitudeTy.getCanonicalType() != SignTy.getCanonicalType()) { + return Diag(Sign.get()->getBeginLoc(), erichkeane wrote: > arsenm wrote: > > arsenm wrote: > > >

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:2674 + +if (MagnitudeTy.getCanonicalType() != SignTy.getCanonicalType()) { + return Diag(Sign.get()->getBeginLoc(), arsenm wrote: > arsenm wrote: > > erichkeane wrote: > > > ar

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

arsenm added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:2674 + +if (MagnitudeTy.getCanonicalType() != SignTy.getCanonicalType()) { + return Diag(Sign.get()->getBeginLoc(), arsenm wrote: > erichkeane wrote: > > arsenm wrote: > > > erichk

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

arsenm added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:2674 + +if (MagnitudeTy.getCanonicalType() != SignTy.getCanonicalType()) { + return Diag(Sign.get()->getBeginLoc(), erichkeane wrote: > arsenm wrote: > > erichkeane wrote: > > > cu

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:2674 + +if (MagnitudeTy.getCanonicalType() != SignTy.getCanonicalType()) { + return Diag(Sign.get()->getBeginLoc(), arsenm wrote: > erichkeane wrote: > > curleys not used for s

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

arsenm added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:2674 + +if (MagnitudeTy.getCanonicalType() != SignTy.getCanonicalType()) { + return Diag(Sign.get()->getBeginLoc(), erichkeane wrote: > curleys not used for single-statement if-sta

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:2660 + +ExprResult Magnitude = UsualUnaryConversions(TheCall->getArg(0)); +ExprResult Sign = UsualUnaryConversions(TheCall->getArg(1)); aaron.ballman wrote: > erichkeane wrote:

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:2660 + +ExprResult Magnitude = UsualUnaryConversions(TheCall->getArg(0)); +ExprResult Sign = UsualUnaryConversions(TheCall->getArg(1)); erichkeane wrote: > arsenm wrote: > >

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

erichkeane added a comment. In D140639#4028900 , @arsenm wrote: > In D140639#4028883 , @erichkeane > wrote: > >> 1 nit, and 1 trying to see what is going on. I don't have a good feeling >> what the purpose of t

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

arsenm added a comment. In D140639#4028883 , @erichkeane wrote: > 1 nit, and 1 trying to see what is going on. I don't have a good feeling > what the purpose of this builtin is, The point of every builtin is direct access to llvm intrinsics, in this c

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

erichkeane added a comment. 1 nit, and 1 trying to see what is going on. I don't have a good feeling what the purpose of this builtin is, nor whether it matches the desire/intent of this builtin, I'm hopeful one of the other reviewers has the ability to check that. Comment

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

arsenm added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140639/new/ https://reviews.llvm.org/D140639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

arsenm created this revision. arsenm added a reviewer: fhahn. Herald added a project: All. arsenm requested review of this revision. Herald added a subscriber: wdng. I realized the handling of copysign made no sense at all. Only the type of the first operand should really matter, and it shou