[PATCH] D146023: [AMDGPU] Remove Code Object V2

Pierre-vh abandoned this revision. Pierre-vh added a comment. https://github.com/llvm/llvm-project/pull/65715 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146023/new/ https://reviews.llvm.org/D146023

[PATCH] D146023: [AMDGPU] Remove Code Object V2

Pierre-vh added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146023/new/ https://reviews.llvm.org/D146023 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D146023: [AMDGPU] Remove Code Object V2

Pierre-vh added inline comments. Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:5598 return ParseDirectiveHSAMetadata(); } else { -if (IDVal == ".hsa_code_object_version") cfang wrote: > Are you sure Non-HSA does not have the four

[PATCH] D146023: [AMDGPU] Remove Code Object V2

Pierre-vh added inline comments. Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:5583 + // FIXME: Shouldn't be needed anymore? Should we remove this directive + // entirely? See `amdpal-elf.ll` - the output ASM contains both amdgcn_target

[PATCH] D152251: [clang][CodeGen] Fix GPU-specific attributes being dropped by bitcode linking

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Pierre-vh marked an inline comment as done. Closed by commit rG23431b524603: [clang][CodeGen] Fix GPU-specific attributes being dropped by bitcode linking (authored by

[PATCH] D152251: [clang][CodeGen] Fix GPU-specific attributes being dropped by bitcode linking

Pierre-vh marked 2 inline comments as done. Pierre-vh added inline comments. Comment at: clang/test/CodeGenCUDA/link-builtin-bitcode-gpu-attrs-preserved.cu:34 +// CHECK: define {{.*}} i32 @do_intrin_stuff() #[[ATTR:[0-9]+]] +// CHECK: attributes #[[ATTR]] = {{.*}}

[PATCH] D146023: [AMDGPU] Remove Code Object V2

Pierre-vh added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp:520 - assert(Func.getCallingConv() == CallingConv::AMDGPU_KERNEL || - Func.getCallingConv() == CallingConv::SPIR_KERNEL); + if (Func.getCallingConv() !=

[PATCH] D152251: [clang][CodeGen] Fix GPU-specific attributes being dropped by bitcode linking

Pierre-vh updated this revision to Diff 528794. Pierre-vh added a comment. Fix check lines, I think it's just FileCheck weirdness Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152251/new/ https://reviews.llvm.org/D152251 Files:

[PATCH] D152251: [clang][CodeGen] Fix GPU-specific attributes being dropped by bitcode linking

Pierre-vh updated this revision to Diff 528793. Pierre-vh marked 3 inline comments as done. Pierre-vh added a comment. target-cpu wasn't set so I tried something a bit different, but now I still need to remove the target-cpu check in the old test. For some reason filecheck doesn't match it but

[PATCH] D152251: [clang][CodeGen] Fix GPU-specific attributes being dropped by bitcode linking

Pierre-vh created this revision. Pierre-vh added reviewers: arsenm, tra, foad. Herald added subscribers: StephenFan, wenlei, tpr. Herald added a project: All. Pierre-vh requested review of this revision. Herald added subscribers: cfe-commits, wdng. Herald added a project: clang. Device libs make

[PATCH] D146023: [AMDGPU] Remove Code Object V2

Pierre-vh added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146023/new/ https://reviews.llvm.org/D146023 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D146023: [AMDGPU] Remove Code Object V2

Pierre-vh removed a reviewer: jdoerfert. Pierre-vh added a comment. Herald added a reviewer: jdoerfert. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146023/new/ https://reviews.llvm.org/D146023

[PATCH] D149986: AMDGPU: Force sc0 and sc1 on stores for gfx940 and gfx941

Pierre-vh added a comment. I think that if this is a new property of the GFX940/941 targets, and turning it off shouldn't be possible, we shouldn't even bother with a feature and just set a bool in the ST for those targets Comment at:

[PATCH] D146945: Add Release Note for -mcode-object-v3 removal

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4559e8e8cd99: Add Release Note for -mcode-object-v3 removal (authored by Pierre-vh). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D146945: Add Release Note for -mcode-object-v3 removal

Pierre-vh added a comment. In D146945#4223677 , @aaron.ballman wrote: > LGTM! Adding the clang-vendors group for awareness since this technically > could break some downstream (I don't expect it to given that this was > deprecated, but you never

[PATCH] D145671: [clang] Remove legacy -m(no)-code-object-v3 options

Pierre-vh added a comment. In D145671#4223655 , @c wrote: > These changes need a release note. Given that this has been deprecated, is > there documentation that should have been removed as well? I can't find anything in the docs about `code-object-v3`

[PATCH] D146945: Add Release Note for -mcode-object-v3 removal

Pierre-vh created this revision. Pierre-vh added a reviewer: aaron.ballman. Herald added a project: All. Pierre-vh requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D146945

[PATCH] D145671: [clang] Remove legacy -m(no)-code-object-v3 options

This revision was automatically updated to reflect the committed changes. Closed by commit rG30e7cd48778b: [clang] Remove legacy -m(no)-code-object-v3 options (authored by Pierre-vh). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145671/new/

[PATCH] D146023: [AMDGPU] Remove Code Object V2

Pierre-vh added inline comments. Comment at: llvm/test/MC/AMDGPU/hsa-gfx10.s:3 -// RUN: llvm-mc -filetype=obj -triple amdgcn--amdhsa -mcpu=gfx1010 --amdhsa-code-object-version=2 -mattr=-wavefrontsize32,+wavefrontsize64 -show-encoding %s | llvm-readobj -S --sd --syms - |

[PATCH] D145671: [clang] Remove legacy -m(no)-code-object-v3 options

Pierre-vh added a comment. I'll wait a couple of more days and then land, if no one has any objections. As said before this has been deprecated for a long time, and clang was already warning on use of those options. Removal has been pending for a while :) Repository: rG LLVM Github Monorepo

[PATCH] D145671: [clang] Remove legacy -m(no)-code-object-v3 options

Pierre-vh created this revision. Pierre-vh added reviewers: arsenm, foad, kzhuravl, rampitec. Herald added subscribers: kosarev, StephenFan, kerbowa, jvesely. Herald added a project: All. Pierre-vh requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, wdng. Herald

[PATCH] D139608: [Clang][NFC] Add default `getBFloat16Mangling` impl

Pierre-vh abandoned this revision. Pierre-vh added a comment. Makes sense to abandon this given that D136919 exists Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139608/new/ https://reviews.llvm.org/D139608

[PATCH] D139608: [Clang][NFC] Add default `getBFloat16Mangling` impl

Pierre-vh added a comment. Ping? it's a small NFC and if it's not desired I don't mind abandoning it; I'd just like to remove this diff from the queue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139608/new/ https://reviews.llvm.org/D139608

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd6acd0196b33: [Sema] Fix crash when evaluating nested call with value-dependent arg (authored by Pierre-vh). Repository: rG LLVM Github Monorepo

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

Pierre-vh updated this revision to Diff 486183. Pierre-vh added a comment. Remove assert Please review @ahatanak Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139713/new/ https://reviews.llvm.org/D139713 Files: clang/lib/AST/ExprConstant.cpp

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

Pierre-vh updated this revision to Diff 482384. Pierre-vh added a comment. Put the assert back in, use alternative fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139713/new/ https://reviews.llvm.org/D139713 Files:

[PATCH] D139398: [AMDGPU] Add bf16 storage support

Pierre-vh added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:4819-4831 +// When we don't have 16 bit instructions, bf16 is illegal and gets +// softened to i16 for storage, with float being used for arithmetic. +// +// After softening,

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

Pierre-vh added a comment. In D139713#3989071 , @shafik wrote: > If I am reading the code correctly it looks like if the call to > `(*I)->isValueDependent()` is true then the temporary will not be created and > therefore we should not be attempting to

[PATCH] D139608: [Clang][NFC] Add default `getBFloat16Mangling` impl

Pierre-vh added a comment. In D139608#3982494 , @jcranmer-intel wrote: > I don't normally handle name mangling, so I can't comment too much here, but > I will note that Itanium ABI is planning on using DF16b for > `std::bfloat16_t`:

[PATCH] D139398: [AMDGPU] Add bf16 storage support

Pierre-vh added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:5573-5576 + SDLoc SL(Op); + return DAG.getNode( + ISD::FP_EXTEND, SL, MVT::f32, + DAG.getNode(ISD::BITCAST, SL, MVT::f16, Op->getOperand(0))); arsenm wrote: >

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

Pierre-vh added a comment. I think the right choice is to remove the assert as it's an invariant that can be broken in some cases, so I don't feel like the assert is worth it anymore. Alternatively I could add something to bypass the assert in that specific call to getParamSlot for

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

Pierre-vh updated this revision to Diff 481626. Pierre-vh added a comment. Missing newline at EOF Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139713/new/ https://reviews.llvm.org/D139713 Files: clang/lib/AST/ExprConstant.cpp

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

Pierre-vh created this revision. Pierre-vh added reviewers: ahatanak, rsmith. Herald added a project: All. Pierre-vh requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix an edge case `ExprConstant.cpp`'s `EvaluateWithSubstitution` when

[PATCH] D139398: [AMDGPU] Add bf16 storage support

Pierre-vh added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:913 + else +RegisterVT = (ScalarVT == MVT::bf16 ? MVT::v2bf16 : MVT::v2f16); IntermediateVT = RegisterVT; arsenm wrote: > If you wanted the promote to i32,

[PATCH] D139398: [AMDGPU] Add bf16 storage support

Pierre-vh added inline comments. Comment at: clang/lib/Basic/Targets/AMDGPU.h:119 + bool hasBFloat16Type() const override { return isAMDGCN(getTriple()); } + const char *getBFloat16Mangling() const override { return "u6__bf16"; }; + arsenm wrote: > Pierre-vh

[PATCH] D139608: [Clang][NFC] Add default `getBFloat16Mangling` impl

Pierre-vh created this revision. Pierre-vh added reviewers: MaskRay, stuij. Herald added subscribers: kosarev, mattd, gchakrabarti, asavonic, StephenFan, kerbowa, jvesely. Herald added a project: All. Pierre-vh requested review of this revision. Herald added subscribers: cfe-commits, jholewinski.

[PATCH] D139398: [AMDGPU] Add bf16 storage support

Pierre-vh added inline comments. Comment at: clang/lib/Basic/Targets/AMDGPU.h:119 + bool hasBFloat16Type() const override { return isAMDGCN(getTriple()); } + const char *getBFloat16Mangling() const override { return "u6__bf16"; }; + Pierre-vh wrote: > arsenm

[PATCH] D139398: [AMDGPU] Add bf16 storage support

Pierre-vh added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:4819-4831 +// When we don't have 16 bit instructions, bf16 is illegal and gets +// softened to i16 for storage, with float being used for arithmetic. +// +// After softening,

[PATCH] D139398: [AMDGPU] Add bf16 storage support

Pierre-vh added inline comments. Comment at: clang/lib/Basic/Targets/AMDGPU.h:119 + bool hasBFloat16Type() const override { return isAMDGCN(getTriple()); } + const char *getBFloat16Mangling() const override { return "u6__bf16"; }; + arsenm wrote: > Don't

[PATCH] D139398: [AMDGPU] Add bf16 storage support

Pierre-vh updated this revision to Diff 480431. Pierre-vh added a comment. - Only accept bf16 on AMDGCN; r600 doesn't support it (we could but it's not worth the effort I think; I'll look at it if we find out it's needed) - Remove bf16 types from a few register classes Repository: rG LLVM

[PATCH] D138651: [CUDA][HIP] Don't diagnose use for __bf16

Pierre-vh abandoned this revision. Pierre-vh added a comment. Added bf16 storage support instead: D139398 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138651/new/ https://reviews.llvm.org/D138651

[PATCH] D139398: [AMDGPU] Add bf16 storage support

Pierre-vh created this revision. Pierre-vh added reviewers: arsenm, foad, yaxunl. Herald added subscribers: kosarev, kerbowa, hiraditya, tpr, dstuttard, jvesely, kzhuravl. Herald added a project: All. Pierre-vh requested review of this revision. Herald added subscribers: llvm-commits,

[PATCH] D138651: [CUDA][HIP] Don't diagnose use for __bf16

Pierre-vh planned changes to this revision. Pierre-vh marked an inline comment as done. Pierre-vh added a comment. I'll take a look at handling bf16 storage-only for AMDGPU. Looks like our Backend already handles it and converts it to i16 so maybe it'll be really easy. Repository: rG LLVM

[PATCH] D138651: [CUDA][HIP] Don't diagnose use for __bf16

Pierre-vh marked an inline comment as done. Pierre-vh added inline comments. Comment at: clang/test/SemaCUDA/amdgpu-bf16.cu:9 + +__device__ void devicefn() { +} tra wrote: > We should probably also have a case verifying that actual attempt to use > `__bf16` in

[PATCH] D138651: [CUDA][HIP] Don't diagnose use for __bf16

Pierre-vh updated this revision to Diff 478823. Pierre-vh marked 5 inline comments as done. Pierre-vh added a comment. - Recentering the patch around HIP only. - I was using too much from D57369 and was involving OpenMP when there's no reason to. Just checking

[PATCH] D138651: [CUDA][HIP] Don't diagnose use for __bf16

Pierre-vh updated this revision to Diff 477871. Pierre-vh added a comment. Fixing condition, adding new test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138651/new/ https://reviews.llvm.org/D138651 Files: clang/lib/AST/ASTContext.cpp

[PATCH] D138651: [CUDA][HIP] Don't diagnose use for __bf16

Pierre-vh updated this revision to Diff 477760. Pierre-vh added a comment. Not all targets have bf16 and AuxTarget may not be available all the time so I changed the condition slightly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138651/new/

[PATCH] D138651: [CUDA][HIP] Don't diagnose use for __bf16

Pierre-vh planned changes to this revision. Pierre-vh added a comment. Need to fix a test crash Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138651/new/ https://reviews.llvm.org/D138651 ___ cfe-commits

[PATCH] D138651: [CUDA][HIP] Don't diagnose use for __bf16

Pierre-vh updated this revision to Diff 477734. Pierre-vh added a comment. Add newline at end of file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138651/new/ https://reviews.llvm.org/D138651 Files: clang/lib/AST/ASTContext.cpp

[PATCH] D138651: [CUDA][HIP] Don't diagnose use for __bf16

Pierre-vh created this revision. Pierre-vh added reviewers: arsenm, rjmccall, tra. Herald added subscribers: kosarev, mattd, kerbowa, pengfei, tpr, yaxunl, jvesely. Herald added a project: All. Pierre-vh requested review of this revision. Herald added subscribers: cfe-commits, sstefan1, wdng.

[PATCH] D137251: [clang][cuda/hip] Allow `__noinline__` lambdas

This revision was automatically updated to reflect the committed changes. Pierre-vh marked 3 inline comments as done. Closed by commit rGc05f1639f7f4: [clang][cuda/hip] Allow `__noinline__` lambdas (authored by Pierre-vh). Changed prior to commit:

[PATCH] D137251: [clang][cuda/hip] Allow `__noinline__` lambdas

Pierre-vh added inline comments. Comment at: clang/lib/Parse/ParseExprCXX.cpp:1300 +ParseGNUAttributes(Attr, nullptr, ); + } else if (Tok.is(tok::kw___noinline__)) { +IdentifierInfo *AttrName = Tok.getIdentifierInfo(); aaron.ballman wrote: >

[PATCH] D137251: [clang][cuda/hip] Allow `__noinline__` lambdas

Pierre-vh updated this revision to Diff 472875. Pierre-vh marked 2 inline comments as done. Pierre-vh added a comment. Comments Not sure if the release note is in the right place though. As for the test, I did something quite targeted/minimal, hope it's fine? Repository: rG LLVM Github

[PATCH] D137251: [clang][cuda/hip] Allow `__noinline__` lambdas

Pierre-vh created this revision. Pierre-vh added reviewers: yaxunl, tra, aaron.ballman, rsmith. Herald added a subscriber: mattd. Herald added a project: All. Pierre-vh requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. D124866

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

Pierre-vh added a comment. Hello! I'm trying one last ping since it's been a month and it hasn't been commited (I think). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55226/new/ https://reviews.llvm.org/D55226 ___ cfe-commits mailing

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

Pierre-vh added a comment. Hello! I'm pinging since it's been a week. If someone can commit this patch on my behalf, that would be great. Thank you :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55226/new/ https://reviews.llvm.org/D55226

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

Pierre-vh marked an inline comment as done. Pierre-vh added a comment. Hi again! As I'm quite new to this, I don't know what the next step is. Do we need to wait for more people to review this diff? What happens when it's considered "ready"? How is it committed? (I don't have commit access)

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

Pierre-vh updated this revision to Diff 176676. Pierre-vh added a comment. Here's the diff without the extra newline :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55226/new/ https://reviews.llvm.org/D55226 Files: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

Pierre-vh updated this revision to Diff 176661. Pierre-vh added a comment. Hello again! I updated the diff and completely removed the outer if. Please let me know what you think! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55226/new/ https://reviews.llvm.org/D55226 Files:

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

Pierre-vh added a comment. In D55226#1317083 , @george.karpenkov wrote: > Thank you for the fix, but how far can the pattern matching go? Seems easy > enough to think of cases not covered by the above. > In any case, the fix looks good. Hey, Sadly

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

Pierre-vh created this revision. Pierre-vh added reviewers: dcoughlin, MaskRay. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, a.sidorin, szepet, baloghadamsoftware. Herald added a reviewer: george.karpenkov. Fix for the bug n°39792: False positive on strcpy targeting