[PATCH] D159206: [Clang] Propagate target-features if compatible when using mlink-builtin-bitcode

jmmartinez added a comment. @yaxunl I've addressed your remarks on a GitHub PR: https://github.com/llvm/llvm-project/pull/65938 Thanks for the remarks! It made me realize that I could simplify the code a lot. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D159257: [Clang] Drop functions with incompatible target-features when using mlink-builtin-bitcode

jmmartinez abandoned this revision. jmmartinez added a comment. Moved into https://github.com/llvm/llvm-project/pull/65737 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159257/new/ https://reviews.llvm.org/D159257 _

[PATCH] D159206: [Clang] Propagate target-features if compatible when using mlink-builtin-bitcode

This revision was automatically updated to reflect the committed changes. Closed by commit rGd60c47476dde: [Clang] Propagate target-features if compatible when using mlink-builtin-bitcode (authored by jmmartinez). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D159206: [Clang] Propagate target-features if compatible when using mlink-builtin-bitcode

jmmartinez added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2030-2031 + bool EnabledForTarget = TEntry->second; + if (EnabledForTarget != EnabledForFunc) +return; +} arsenm wrote: > jmmartinez wrote: > > arsenm wrote: > > > Ea

[PATCH] D159206: [Clang] Propagate target-features if compatible when using mlink-builtin-bitcode

jmmartinez updated this revision to Diff 555987. jmmartinez marked 2 inline comments as done. jmmartinez added a comment. - Capitalize comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159206/new/ https://reviews.llvm.org/D159206 Files: clan

[PATCH] D159206: [Clang] Propagate target-features if compatible when using mlink-builtin-bitcode

jmmartinez updated this revision to Diff 555986. jmmartinez marked an inline comment as done. jmmartinez added a comment. - Review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159206/new/ https://reviews.llvm.org/D159206 Files: clang/lib/CodeGe

[PATCH] D159206: [Clang] Propagate target-features if compatible when using mlink-builtin-bitcode

jmmartinez added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2017 +for (StringRef Feature : llvm::split(FFeatures.getValueAsString(), ',')) { + bool EnabledForFunc = Feature[0] == '+'; + StringRef Name = Feature.substr(1); arsenm wrote

[PATCH] D159206: [Clang] Propagate target-features if compatible when using mlink-builtin-bitcode

jmmartinez updated this revision to Diff 555691. jmmartinez marked 4 inline comments as done. jmmartinez added a comment. - Review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159206/new/ https://reviews.llvm.org/D159206 Files: clang/lib/CodeGe

[PATCH] D159206: [Clang] Propagate target-features if compatible when using mlink-builtin-bitcode

jmmartinez updated this revision to Diff 555051. jmmartinez added a comment. - Rebased Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159206/new/ https://reviews.llvm.org/D159206 Files: clang/lib/CodeGen/CGCall.cpp clang/test/CodeGen/link-built

[PATCH] D159256: [NFC][Clang] Remove redundant function definitions

jmmartinez added a comment. Thanks for the review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159256/new/ https://reviews.llvm.org/D159256 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D159256: [NFC][Clang] Remove redundant function definitions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG19550e79b50f: [NFC][Clang] Remove redundant function definitions (authored by jmmartinez). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D159256: [NFC][Clang] Remove redundant function definitions

jmmartinez added a comment. In D159256#4630876 , @jhuber6 wrote: > In D159256#4630410 , @jmmartinez > wrote: > >> @jhuber6 I was wondering if there is a reason you kept 3 versions of >> `mergeDefaultFunctionDefi

[PATCH] D159257: [Clang] Drop functions with incompatible target-features when using mlink-builtin-bitcode

jmmartinez created this revision. jmmartinez added reviewers: arsenm, Pierre-vh, yaxunl, jhuber6. Herald added a project: All. jmmartinez requested review of this revision. Herald added subscribers: cfe-commits, wdng. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://re

[PATCH] D159206: [Clang] Propagate target-features if compatible when using mlink-builtin-bitcode

jmmartinez updated this revision to Diff 554934. jmmartinez added a comment. - Rollback, drop incompatible functions in a separate commit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159206/new/ https://reviews.llvm.org/D159206 Files: clang/lib

[PATCH] D159256: [NFC][Clang] Remove redundant function definitions

jmmartinez added a comment. @jhuber6 I was wondering if there is a reason you kept 3 versions of `mergeDefaultFunctionDefinitionAttributes` in https://reviews.llvm.org/D152391 ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159256/new/ https://rev

[PATCH] D159256: [NFC][Clang] Remove redundant function definitions

jmmartinez created this revision. jmmartinez added a reviewer: jhuber6. Herald added a project: All. jmmartinez requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. There were 3 definitions of the mergeDefaultFunctionDefinitionAttributes functio

[PATCH] D159206: [Clang] Propagate target-features if compatible when using mlink-builtin-bitcode

jmmartinez added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:282 } +} Remove { } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159206/new/ https://reviews.llvm.org/D159206

[PATCH] D159206: [Clang] Propagate target-features if compatible when using mlink-builtin-bitcode

jmmartinez updated this revision to Diff 554753. jmmartinez marked an inline comment as done. jmmartinez added a comment. - Drop incoming builtins with incompatible target-features Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159206/new/ https://r

[PATCH] D159206: [Clang] Propagate target-features if compatible when using mlink-builtin-bitcode

jmmartinez marked 2 inline comments as done. jmmartinez added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2034 + } + + FuncAttr.addAttribute("target-features", llvm::join(MergedFeatures, ",")); arsenm wrote: > Really it would be less bad if the in

[PATCH] D159206: [Clang] Propagate target-features if compatible when using mlink-builtin-bitcode

jmmartinez created this revision. jmmartinez added reviewers: arsenm, Pierre-vh, yaxunl, jhuber6. Herald added a project: All. jmmartinez requested review of this revision. Herald added subscribers: cfe-commits, wdng. Herald added a project: clang. Buitlins from AMD's device-libs are compiled with

[PATCH] D158990: [NFC][Clang] Remove unused function `CodeGenModule::addDefaultFunctionDefinitionAttributes`

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG9b352540184e: [NFC][Clang] Remove unused function `CodeGenModule… (authored by jmmartinez). Repository: rG LLVM Github

[PATCH] D158991: [NFC][Clang] Add missing & to function argument

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGb63c6e585d86: [NFC][Clang] Add missing & to function argument (authored by jmmartinez). Repository: rG LLVM Github Mono

[PATCH] D158991: [NFC][Clang] Add missing & to function argument

jmmartinez created this revision. Herald added a project: All. jmmartinez 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/D158991 Files: clang/lib/CodeGen/CGCall.cpp Index

[PATCH] D158990: [NFC][Clang] Remove unused function `CodeGenModule::addDefaultFunctionDefinitionAttributes`

jmmartinez created this revision. Herald added a project: All. jmmartinez 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/D158990 Files: clang/lib/CodeGen/CGCall.cpp clang

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

jmmartinez added inline comments. Comment at: llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp:524 + SIAtomicAddrSpace::NONE) + return enableSC0Bit(MI) | enableSC1Bit(MI); +return false; NIT: Is the use of the bitwise or " | " intended? I'd use the l

[PATCH] D144870: [Clang][DebugInfo] Emit zero size bitfields in the debug info to delimit bitfields in different allocation units.

jmmartinez added a comment. In D144870#4225437 , @probinson wrote: > One entirely optional suggestion on the test. LGTM. Thanks! I used `SECONDDUP` and `FIRSTDUP`. And thanks a lot for the reviews! Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D144870: [Clang][DebugInfo] Emit zero size bitfields in the debug info to delimit bitfields in different allocation units.

This revision was automatically updated to reflect the committed changes. Closed by commit rG488185cca387: [Clang][DebugInfo][AMDGPU] Emit zero size bitfields in the debug info to… (authored by jmmartinez). Changed prior to commit: https://reviews.llvm.org/D144870?vs=508538&id=508926#toc Repos

[PATCH] D144870: [Clang][DebugInfo] Emit zero size bitfields in the debug info to delimit bitfields in different allocation units.

jmmartinez added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1563 + + assert(PreviousBitfield->isBitField()); + probinson wrote: > Is this true for cases like > ``` > struct nonadjacent { > char a : 8; > char : 0; > int b; > char d : 8

[PATCH] D144870: [Clang][DebugInfo] Emit zero size bitfields in the debug info to delimit bitfields in different allocation units.

jmmartinez updated this revision to Diff 508538. jmmartinez marked 3 inline comments as done. jmmartinez added a comment. - Took remarks into account Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144870/new/ https://reviews.llvm.org/D144870 Files:

[PATCH] D144870: [Clang][DebugInfo] Emit zero size bitfields in the debug info to delimit bitfields in different allocation units.

jmmartinez updated this revision to Diff 506611. jmmartinez added a comment. - Updated to use `elements` array to avoid the complicated loop. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144870/new/ https://reviews.llvm.org/D144870 Files: clang

[PATCH] D144870: [Clang][DebugInfo] Emit zero size bitfields in the debug info to delimit bitfields in different allocation units.

jmmartinez added a comment. In D144870#4202121 , @probinson wrote: > (Of course it's possible that finding the preceding field can be done only by > iterating, but I'd hope that isn't necessary.) Sadly, this is the case. `field_iterator` and the underly

[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (5/7)

jmmartinez accepted this revision. jmmartinez added inline comments. This revision is now accepted and ready to land. Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:712-714 + assert(!getAbstractScopeDIEs().count(DS) && + "Abstract DIE for this scope

[PATCH] D144870: [Clang][DebugInfo] Emit zero size bitfields in the debug info to delimit bitfields in different allocation units.

jmmartinez added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1558 + EmitSeparator = FieldIt->isBitField(); + } + probinson wrote: > I might not be following this correctly, but it feels like EmitSeparator will > end up true if the last f

[PATCH] D144870: [Clang][DebugInfo] Emit zero size bitfields in the debug info to delimit bitfields in different allocation units.

jmmartinez updated this revision to Diff 505507. jmmartinez added a comment. - Fixed the loop and added test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144870/new/ https://reviews.llvm.org/D144870 Files: clang/lib/CodeGen/CGDebugInfo.cpp

[PATCH] D144870: [Clang][DebugInfo] Emit zero size bitfields in the debug info to delimit bitfields in different allocation units.

jmmartinez updated this revision to Diff 505148. jmmartinez added a comment. Update: - Took into account remarks and got my ideas straight. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144870/new/ https://reviews.llvm.org/D144870 Files: clang/

[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (5/7)

jmmartinez added a comment. Just a few minor comments. Everything else seems good to me. Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:698 + // an inlined function: if a local variable has a templated type with + // a function-local type as a template parameter

[PATCH] D143984: [DebugMetadata] Simplify handling subprogram's retainedNodes field. NFCI (1/7)

jmmartinez added a comment. Sorry for the delay, I was out-of-office. Patches 1 to 4 look good to me. If nobody raises concerns this week I'll mark them as accepted. I'm currently reviewing patch 5. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14

[PATCH] D144870: [Clang][DebugInfo] Emit zero size bitfields in the debug info to delimit bitfields in different allocation units.

jmmartinez created this revision. jmmartinez added reviewers: dblaikie, probinson. Herald added subscribers: kosarev, tpr. Herald added a project: All. jmmartinez requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. I'm looking for some feedback

[PATCH] D143984: [DebugMetadata] Simplify handling subprogram's retainedNodes field. NFCI (1/7)

jmmartinez added inline comments. Comment at: llvm/lib/IR/DIBuilder.cpp:789 + return createLocalVariable( + VMContext, getSubprogramNodesTrackingVector(Scope), Scope, Name, + /* ArgNo */ 0, File, LineNo, Ty, AlwaysPreserve, Flags, AlignInBits); krisb w

[PATCH] D144004: [DebugMetadata][DwarfDebug] Fix DWARF emisson of function-local imported entities (3/7)

jmmartinez added a comment. I've added just a few minor remarks. Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:1082-1084 + if (!includeMinimalInlineScopes() && !Scope->getInlinedAt()) +for (const auto *Decl : DD->getLocalDeclsForScope(Scope->getScopeNode()))

[PATCH] D143984: [DebugMetadata] Simplify handling subprogram's retainedNodes field. NFCI (1/7)

jmmartinez added inline comments. Comment at: llvm/lib/IR/DIBuilder.cpp:789 + return createLocalVariable( + VMContext, getSubprogramNodesTrackingVector(Scope), Scope, Name, + /* ArgNo */ 0, File, LineNo, Ty, AlwaysPreserve, Flags, AlignInBits); I'll be

[PATCH] D141700: AMDGPU: Move enqueued block handling into clang

jmmartinez added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:12581 + Mod, HandleTy, + /*isConstant=*/true, llvm::GlobalValue::InternalLinkage, + /*Initializer=*/RuntimeHandleInitializer, RuntimeHandleName, Just a cosmetical remark

[PATCH] D140195: [Clang][CGDebugInfo][ObjC] Mark objc bitfields with the DIFlagBitfield flag

This revision was automatically updated to reflect the committed changes. Closed by commit rG1118ee04fc7c: [Clang][CGDebugInfo][ObjC] Mark objc bitfields with the DIFlagBitfield flag (authored by jmmartinez). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D140195: [Clang][CGDebugInfo][ObjC] Mark objc bitfields with the DIFlagBitfield flag

jmmartinez added a comment. In D140195#4008724 , @aprantl wrote: >> Unluckly, I wasn't able to test the combination of >> https://reviews.llvm.org/D96334 and this patch together. If you have some >> pointers about how to trigger a test job on "Green Dra

[PATCH] D140195: [Clang][CGDebugInfo][ObjC] Mark objc bitfields with the DIFlagBitfield flag

jmmartinez added a comment. In D140195#4001989 , @aprantl wrote: > How does this affect the generated DWARF? Have you tested that this does the > right thing in LLDB? It shouldn't affect the generated DWARF on LLVM's main branch. Currently, LLVM relies

[PATCH] D140195: [Clang][CGDebugInfo][ObjC] Mark objc bitfields with the DIFlagBitfield flag

jmmartinez updated this revision to Diff 483468. jmmartinez added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140195/new/ https://reviews.llvm.org/D140195 Files: clang/lib/CodeGen/CGDebugInfo.cpp clang/test/CodeGenObj

[PATCH] D140195: [Clang][CGDebugInfo][ObjC] Mark objc bitfields with the DIFlagBitfield flag

jmmartinez created this revision. Herald added a project: All. jmmartinez 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/D140195 Files: clang/lib/CodeGen/CGDebugInfo.cpp

[PATCH] D139023: [NFC][Clang][Driver][AMDGPU] Avoid temporary copies of std::string by using Twine and StringRef

jmmartinez added a comment. @tra Thanks for the review! I double checked with ASAN and there were no issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139023/new/ https://reviews.llvm.org/D139023 ___

[PATCH] D139023: [NFC][Clang][Driver][AMDGPU] Avoid temporary copies of std::string by using Twine and StringRef

This revision was automatically updated to reflect the committed changes. Closed by commit rGa446827249bd: [NFC][Clang][Driver][AMDGPU] Avoid temporary copies of std::string by using… (authored by jmmartinez). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D139023: [NFC][Clang][Driver][AMDGPU] Avoid temporary copies of std::string by using Twine and StringRef

jmmartinez created this revision. Herald added subscribers: kosarev, kerbowa, tpr, dstuttard, yaxunl, jvesely, kzhuravl. Herald added a project: All. jmmartinez requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, wdng. Herald added a project: clang. Repository: rG