[PATCH] D149800: [WIP][PGO] Add ability to mark cold functions as optsize/minsize/optnone

mtrofin added a comment. high level question, why not have it as a pass that runs after profiles (of whatever kind - instrumented or sample-based) are ingested. The pass would attribute functions as described. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

mtrofin added a comment. In D143624#4315508 , @nikic wrote: > In D143624#4315468 , @dmgreen wrote: > >> It looks like there is quite a lot more optimization that happens to the >> function being always-inlined (_

[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

mtrofin added a comment. In D143624#4116546 , @aeubanks wrote: > Had a chat offline with @mtrofin, wanted to be clear for future purposes that > we do need the separate AlwaysInliner pass because it's used in -O0 and > constructing a call graph there is

[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

mtrofin accepted this revision. mtrofin added a comment. This revision is now accepted and ready to land. lgtm. Like @aeubanks was saying, let's just give a bit of time (1 month or so?) between when this lands and until we clean up the "mandatory" notion from the advisor, just to make sure nothi

[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

mtrofin added a comment. In D143624#4115986 , @aeubanks wrote: > __clang_hip_math.hip is annoying... > > We'll need to remove the `MandatoryFirst` inliner in > `ModuleInlinerWrapperPass`, although not sure if @mtrofin has any issues with > that or not

[PATCH] D139006: [UpdateTestChecks] Match define for labels

mtrofin accepted this revision. mtrofin added a comment. LGTM, but I'd wait for Johannes to review it, too (because of e67f6477fd1ed29acbeddf8482c25d8db826912f . I for one don't quite follow the reasoning there wrt adding the

[PATCH] D138836: [UpdateTestChecks] Fix `update_test_checks.py` to add "unused" prefixes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG255e7e1c21ee: [UpdateTestChecks] Fix `update_*_test_checks.py` to add "unused" prefixes (authored by mtrofin). Repository: rG LLVM Github Monorep

[PATCH] D138836: [UpdateTestChecks] Fix `update_test_checks.py` to add "unused" prefixes

mtrofin added a comment. In D138836#3954851 , @lebedev.ri wrote: > In D138836#3954850 , @mtrofin wrote: > >> In D138836#3954782 , @lebedev.ri >> wrote: >> >>> Thanks! >>

[PATCH] D138836: [UpdateTestChecks] Fix `update_test_checks.py` to add "unused" prefixes

mtrofin added a comment. In D138836#3954782 , @lebedev.ri wrote: > Thanks! > There's also the one for MCA, but this situation basically never happens for > those tests. Yup, checked that and the mir one. The latter seems to be implementing its own "ad

[PATCH] D138836: [UpdateTestChecks] Fix `update_test_checks.py` to add "unused" prefixes

mtrofin updated this revision to Diff 478343. mtrofin added a comment. Herald added a subscriber: pcwang-thead. missed one Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138836/new/ https://reviews.llvm.org/D138836 Files: clang/test/utils/update

[PATCH] D138836: [UpdateTestChecks] Fix `update_test_checks.py` to add "unused" prefixes

mtrofin updated this revision to Diff 478339. mtrofin added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. fixed the other 2 scripts Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138836/new/ https://reviews.llvm.o

[PATCH] D132991: [Clang] Give error message for invalid profile path when compiling IR

mtrofin added a comment. In D132991#3764877 , @aidengrossman wrote: > @xur I've modified the patch slightly (mainly fixing tests and changing the > error message printing in `CodeGenModule` to an assert as we should be > capturing everything in `Compil

[PATCH] D131448: Introduce iterator sentinel to make graph traversal implementation more efficient and cleaner

mtrofin added a comment. lgtm for the MLInlineAdvisor bit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131448/new/ https://reviews.llvm.org/D131448 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGafb4efd3bcc6: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled (authored by aidengrossman, committed by mtrofin). Repository:

[PATCH] D130620: Fix lack of cc1 flag in llvmcmd sections when assertions are enabled

mtrofin added a comment. In D130620#3686357 , @aidengrossman wrote: > Thanks for the review and your suggestions. Do you mind pushing this commit? > I don't currently have commit access to LLVM. Thanks. I can do it. Repository: rG LLVM Github Monor

[PATCH] D119876: [nfc][codegen] Move RegisterBank[Info].h under CodeGen

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. mtrofin marked an inline comment as done. Closed by commit rGcb2160760e67: [nfc][codegen] Move RegisterBank[Info].h under CodeGen (authored by mtrofin). Herald added a

[PATCH] D119876: [nfc][codegen] Move RegisterBank[Info].h under CodeGen

mtrofin marked an inline comment as done. mtrofin added inline comments. Comment at: llvm/lib/Target/ARM/ARMTargetMachine.cpp:43 #include "llvm/Pass.h" +#include "llvm/Support/ARMTargetParser.h" #include "llvm/Support/CodeGen.h" myhsu wrote: > Hmm...did you use

[PATCH] D119876: [nfc][codegen] Move RegisterBank[Info].h under CodeGen

mtrofin added a comment. As this is a follow-up of a refactoring, I assume I can just land it (short of file header comments, there was nothing really this patch did more intelligently) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119876/new/ ht

[PATCH] D119876: [nfc][codegen] Move RegisterBank[Info].h under CodeGen

mtrofin added inline comments. Comment at: llvm/include/llvm/CodeGen/RegisterBankInfo.h:220 : ID(ID), Cost(Cost), OperandsMapping(OperandsMapping), - NumOperands(NumOperands) { -} + NumOperands(NumOperands) {} ah - `clang-format`

[PATCH] D119876: [nfc][codegen] Move RegisterBank[Info].h under CodeGen

mtrofin created this revision. mtrofin added a reviewer: qcolombet. Herald added subscribers: foad, frasercrmck, kerbowa, luismarques, apazos, sameer.abuasal, pengfei, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, atanasyan, edward-jones, zzheng, jrtc27, niosHD, sabu

[PATCH] D115393: [InstrProf][NFC] Refactor Profile kind into a bitset enum.

mtrofin added inline comments. Comment at: llvm/include/llvm/ProfileData/InstrProfReader.h:495 + InstrProfKind getProfileKind() const override { +InstrProfKind ProfileKind = InstrProfKind::Unknown; This looks a lot like line 290, can it be refactored (or a

[PATCH] D79959: [SampleFDO] Add use-sample-profile function attribute

mtrofin added inline comments. Herald added subscribers: ormris, jdoerfert. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:856 + // Add use-sample-profile value. + if (!CGM.getCodeGenOpts().SampleProfileFile.empty()) MaskRay wrote: > The code self explains

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

mtrofin added inline comments. Comment at: llvm/lib/Transforms/IPO/Inliner.cpp:1017 +// invalidate analyses for all functions in this SCC later. +FAM.invalidate(F, PreservedAnalyses::none()); } Should we do this if !Changed? Actually, if this function

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

This revision was automatically updated to reflect the committed changes. mtrofin marked an inline comment as done. Closed by commit rG7d541eb4d49a: [inliner] Mandatory inlining decisions produce remarks (authored by mtrofin). Changed prior to commit: https://reviews.llvm.org/D110891?vs=377091&

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

mtrofin marked an inline comment as done. mtrofin added inline comments. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:72-89 + void recordUnsuccessfulInliningImpl(const InlineResult &Result) override { +if (IsInliningRecommended) + ORE.emit([&]() { +return Opt

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

mtrofin updated this revision to Diff 377091. mtrofin marked 2 inline comments as done. mtrofin added a comment. added test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110891/new/ https://reviews.llvm.org/D110891 Files: clang/test/Frontend/opt

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

mtrofin marked 3 inline comments as done. mtrofin added inline comments. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:52 +namespace { +using namespace llvm::ore; wenlei wrote: > mtrofin wrote: > > wenlei wrote: > > > curious why do we need anonymous namespac

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

mtrofin marked an inline comment as done. mtrofin added inline comments. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:52 +namespace { +using namespace llvm::ore; wenlei wrote: > curious why do we need anonymous namespace here? iiuc it's preferred we place fi

[PATCH] D110891: [inliner] Mandatory inlining decisions produce remarks

mtrofin created this revision. mtrofin added a reviewer: aeubanks. Herald added subscribers: ormris, wenlei, hiraditya, eraman. mtrofin requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. This also removes the need to disable

[PATCH] D107025: Take OptimizationLevel class out of Pass Builder

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7a797b290299: Take OptimizationLevel class out of Pass Builder (authored by tarinduj, committed by mtrofin). Repository: rG LLVM Github Monorepo

[PATCH] D107025: Take OptimizationLevel class out of Pass Builder

mtrofin accepted this revision. mtrofin added inline comments. This revision is now accepted and ready to land. Comment at: llvm/include/llvm/Passes/OptimizationLevel.h:128 +#endif \ No newline at end of file add a newline here (it helps diff tools) CHANGES SI

[PATCH] D107025: Take OptimizationLevel class out of Pass Builder

mtrofin added inline comments. Comment at: llvm/include/llvm/Passes/OptimizationLevel.h:14 +//===--===// + +class OptimizationLevel final { this should be in the llvm namespace

[PATCH] D100917: [NewPM] Only invalidate modified functions' analyses in CGSCC passes

mtrofin added a comment. In D100917#2736702 , @aeubanks wrote: > In D100917#2735651 , @nikic wrote: > >> An unfortunate side-effect of this change is that NewPM uses even more >> memory. tramp3d-v4 is up 20% in m

[PATCH] D98440: [NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function passes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5eaeb0fa67e5: [NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function… (authored by mtrofin). Repository: rG LLVM Github

[PATCH] D98440: [NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function passes

mtrofin added a comment. In D98440#2620437 , @aeubanks wrote: > This doesn't break the pipeline tests in llvm/test/Other? > Running check-llvm with expensive checks is probably a good idea to see if > there are any weird issues. Done - nothing else broke

[PATCH] D98440: [NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function passes

mtrofin updated this revision to Diff 330057. mtrofin marked an inline comment as done. mtrofin added a comment. feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98440/new/ https://reviews.llvm.org/D98440 Files: clang/test/CodeGen/thinlto-

[PATCH] D98440: [NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function passes

mtrofin created this revision. mtrofin added reviewers: asbirlea, aeubanks. Herald added subscribers: steven_wu, hiraditya. mtrofin requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Check with the analysis result by calling

[PATCH] D95849: [FileCheck] Default --allow-unused-prefixes to false

mtrofin added a comment. In D95849#2535939 , @jhenderson wrote: > I assume this now runs cleanly. If so, LGTM. Probably worth mentioning on the > mailing list thread too, so wrap up that conversation. By the way, did we > ever figure out how many of the

[PATCH] D95842: [NFC] Remove unused prefixes under clang/test/OpenMP

mtrofin created this revision. mtrofin added a reviewer: jdoerfert. Herald added subscribers: guansong, yaxunl. mtrofin requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. The full list of pertinent tests is larger, so chunking the cha

[PATCH] D95660: [NFC] Disallow unused prefixes under clang/test/Driver

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc4d6f2707a1e: [NFC] Disallow unused prefixes under clang/test/Driv

[PATCH] D95660: [NFC] Disallow unused prefixes under clang/test/Driver

mtrofin updated this revision to Diff 320256. mtrofin added a comment. indent Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95660/new/ https://reviews.llvm.org/D95660 Files: clang/test/Driver/amdgpu-macros.cl clang/test/Driver/cuda-detect.cu

[PATCH] D95660: [NFC] Disallow unused prefixes under clang/test/Driver

mtrofin added inline comments. Comment at: clang/test/Driver/rocm-device-libs.cl:82 // RUN: %s \ -// RUN: 2>&1 | FileCheck --check-prefixes=COMMON,COMMON-UNSAFE,GFX803,WAVE64 %s +// RUN: 2>&1 | FileCheck --check-prefixes=COMMON,GFX803,WAVE64 %s MaskRay wr

[PATCH] D95660: [NFC] Disallow unused prefixes under clang/test/Driver

mtrofin updated this revision to Diff 320246. mtrofin marked an inline comment as done. mtrofin added a comment. cleaned up the RUN line for rocm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95660/new/ https://reviews.llvm.org/D95660 Files: cla

[PATCH] D95660: [NFC] Disallow unused prefixes under clang/test/Driver

mtrofin created this revision. Herald added subscribers: kerbowa, mstorsjo, nhaehnle, jvesely. mtrofin 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/D95660 Files: clang/te

[PATCH] D95499: [NFC] Disallow unused prefixes under clang/test/CodeGenCXX

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcfcc1110d773: [NFC] Disallow unused prefixes under clang/test/CodeGenCXX (authored by mtrofin). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D95499: [NFC] Disallow unused prefixes under clang/test/CodeGenCXX

mtrofin created this revision. mtrofin added a reviewer: rnk. mtrofin requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The only test that needed change had 'QUAL' as an unused prefix. The rest of the changes are to simplify the prefix lists.

[PATCH] D95417: [NFC] Disallow unused prefixes under clang/test/CodeGen

This revision was automatically updated to reflect the committed changes. Closed by commit rG0c0d009a88f2: [NFC] Disallow unused prefixes under clang/test/CodeGen (authored by mtrofin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95417/new/ https:

[PATCH] D95417: [NFC] Disallow unused prefixes under clang/test/CodeGen

mtrofin created this revision. mtrofin added a reviewer: MaskRay. mtrofin requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D95417 F

[PATCH] D95249: [NFC] Disallow unused prefixes in clang/test/Analysis

This revision was automatically updated to reflect the committed changes. Closed by commit rG91b61abafb5a: [NFC] Disallow unused prefixes in clang/test/Analysis (authored by mtrofin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95249/new/ https://

[PATCH] D95249: [NFC] Disallow unused prefixes in clang/test/Analysis

mtrofin updated this revision to Diff 319024. mtrofin added a comment. fixed trimmers.dot Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95249/new/ https://reviews.llvm.org/D95249 Files: clang/test/Analysis/auto-obj-dtors-cfg-output.cpp clang/t

[PATCH] D95249: [NFC] Disallow unused prefixes in clang/test/Analysis

mtrofin created this revision. mtrofin 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/D95249 Files: clang/test/Analysis/auto-obj-dtors-cfg-output.cpp clang/test/Analysis/

[PATCH] D94825: [NewPM][Inliner] Move the 'always inliner' case in the same CGSCC pass as 'regular' inliner

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe8049dc3c8a4: [NewPM][Inliner] Move the 'always inliner' case in the same CGSCC pass as… (authored by mtrofin). Repository: rG LLVM Github Monorep

[PATCH] D94825: [NewPM][Inliner] Move the 'always inliner' case in the same CGSCC pass as 'regular' inliner

mtrofin updated this revision to Diff 317133. mtrofin added a comment. Herald added subscribers: wenlei, steven_wu. updated with all tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94825/new/ https://reviews.llvm.org/D94825 Files: clang/test

[PATCH] D94825: [NewPM][Inliner] Move the 'always inliner' case in the same CGSCC pass as 'regular' inliner

mtrofin marked 2 inline comments as done. mtrofin added inline comments. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:463 +return getAdviceImpl(CB); + bool Advice = CB.getCaller() != CB.getCalledFunction() && +MandatoryInliningKind::Always == -

[PATCH] D94825: [NewPM][Inliner] Move the 'always inliner' case in the same CGSCC pass as 'regular' inliner

mtrofin marked 2 inline comments as done. mtrofin added inline comments. Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:463 +return getAdviceImpl(CB); + bool Advice = CB.getCaller() != CB.getCalledFunction() && +MandatoryInliningKind::Always == -

[PATCH] D94825: [NewPM]i[Inliner] Move the 'always inliner' case in the same CGSCC pass as 'regular' inliner

mtrofin updated this revision to Diff 317085. mtrofin added a comment. patched tests other than Other/ ones. We want to rationalize emitting remarks, see also D94334 ; I'd prefer first landing the always inlining refactoring, because that would impact the remark

[PATCH] D94825: [NewPM]i[Inliner] Move the 'always inliner' case in the same CGSCC pass as 'regular' inliner

mtrofin created this revision. mtrofin added a reviewer: aeubanks. Herald added subscribers: hiraditya, eraman. mtrofin requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Expanding from D94808

[PATCH] D94808: [NewPM][Inliner] Move mandatory inliner inside function simplification pipeline

mtrofin added a comment. I think InlineAdvisor::getAdvice needs to take a bool MandatoryOnly, which Inliner then passes. This allows us to then use the same advisor for both the always case and the policy-driven inliner instances, which allows that advisor to correctly book-keep any module wide

[PATCH] D93078: [utils] Fix UpdateTestChecks case where 2 runs differ for last label

mtrofin added a comment. In D93078#2500124 , @jdoerfert wrote: > In D93078#2500122 , @mtrofin wrote: > >> In D93078#2500114 , @jdoerfert >> wrote: >> >>> In D93078#2500040 <

[PATCH] D93078: [utils] Fix UpdateTestChecks case where 2 runs differ for last label

mtrofin added a comment. In D93078#2500114 , @jdoerfert wrote: > In D93078#2500040 , @mtrofin wrote: > >> In D93078#2500032 , @jdoerfert >> wrote: >> >>> In D93078#246 <

[PATCH] D93078: [utils] Fix UpdateTestChecks case where 2 runs differ for last label

mtrofin added a comment. In D93078#2500032 , @jdoerfert wrote: > In D93078#246 , @mtrofin wrote: > >> In D93078#245 , @jdoerfert >> wrote: >> >>> In D93078#2499982 <

[PATCH] D93078: [utils] Fix UpdateTestChecks case where 2 runs differ for last label

mtrofin added a comment. In D93078#245 , @jdoerfert wrote: > In D93078#2499982 , @mtrofin wrote: > >> In D93078#2499666 , @jdoerfert >> wrote: >> >>> I think this caused

[PATCH] D93078: [utils] Fix UpdateTestChecks case where 2 runs differ for last label

mtrofin added a comment. In D93078#2499666 , @jdoerfert wrote: > I think this caused a lot of problems for the Attributor tests. I get the > "conflict output" warning all the time now :( > > These two `UpdateTestChecks` tests also generate the warning. I

[PATCH] D94644: [Inliner] Inline alwaysinline calls first

mtrofin added a comment. In D94644#2498638 , @aeubanks wrote: > It's not an emergency. > The issue with InlineAdvisorAnalysis is that the ModuleInlineWrapperPass > presets the InliningAdvisorMode of the InlineAdvisorAnalysis. We could force > override it

[PATCH] D94644: [Inliner] Inline alwaysinline calls first

mtrofin added a comment. In D94644#2497180 , @aeubanks wrote: > An alternative is to run the mandatory inliner in the same CGSCC pipeline as > everything else, but the way InlineAdvisorAnalysis is setup made it hard to > implement I don't remember there

[PATCH] D94644: [Inliner] Inline alwaysinline calls first

mtrofin added a comment. Would running the function simplification pipeline after the always inline pass address the issue? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94644/new/ https://reviews.llvm.org/D94644 _

[PATCH] D93078: [utils] Fix UpdateTestChecks case where 2 runs differ for last label

mtrofin added inline comments. Comment at: llvm/utils/update_analyze_test_checks.py:129 + +common.warn_on_failed_prefixes(func_dict) is_in_function = False pengfei wrote: > Can we move these warn to common.py? Come to think of it, maybe moving it to

[PATCH] D93078: [utils] Fix UpdateTestChecks case where 2 runs differ for last label

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe2dc306b1ac7: [utils] Fix UpdateTestChecks case where 2 runs differ for last label (authored by mtrofin). Repository: rG LLVM Github Monorepo CHA

[PATCH] D93078: [utils] Fix UpdateTestChecks case where 2 runs differ for last label

mtrofin updated this revision to Diff 311896. mtrofin added a comment. update_test_prefix.py change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93078/new/ https://reviews.llvm.org/D93078 Files: clang/test/utils/update_cc_test_checks/Inputs/pre

[PATCH] D93078: [utils] Fix UpdateTestChecks case where 2 runs differ for last label

mtrofin added a comment. In D93078#2453891 , @pengfei wrote: > LGTM. Thanks. > `update_test_prefix.py` assumes the conflicting output. You may need to > change the expection of it as well. oh yes - made it check the new warning. Thanks! Repository: r

[PATCH] D93078: [utils] Fix UpdateTestChecks case where 2 runs differ for last label

mtrofin added a comment. cleanup Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93078/new/ https://reviews.llvm.org/D93078 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D93078: [utils] Fix UpdateTestChecks case where 2 runs differ for last label

mtrofin updated this revision to Diff 311593. mtrofin marked 3 inline comments as done. mtrofin added a comment. fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93078/new/ https://reviews.llvm.org/D93078 Files: clang/test/utils/update_cc_test_

[PATCH] D93078: [utils] Fix UpdateTestChecks case where 2 runs differ for last label

mtrofin marked an inline comment as done. mtrofin added a comment. In D93078#2451747 , @pengfei wrote: > What's the difference with the existing code? It looks to me that you just > brought the warning out of loop, right? Oh true! we can just do the chec

[PATCH] D93078: [utils] Fix UpdateTestChecks case where 2 runs differ for last label

mtrofin updated this revision to Diff 311359. mtrofin marked an inline comment as done. mtrofin added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. added check for when a prefix has conflicts for all functions Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5fe10263ab39: [llvm][inliner] Reuse the inliner pass to implement 'always inliner' (authored by mtrofin). Repository: rG LLVM Github Monorepo CHA

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

mtrofin added inline comments. Comment at: llvm/include/llvm/Analysis/InlineAdvisor.h:27 /// There are 3 scenarios we can use the InlineAdvisor: /// - Default - use manual heuristics. aeubanks wrote: > aeubanks wrote: > > 4 > ping sorry - done Repository:

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

mtrofin updated this revision to Diff 308441. mtrofin marked 5 inline comments as done. mtrofin added a comment. fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91567/new/ https://reviews.llvm.org/D91567 Files: clang/test/CodeGen/thinlto-dis

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

mtrofin updated this revision to Diff 308422. mtrofin added a comment. Fixed the LTO case. Also fixed the p46945 test, which, post - D90566 , was passing without the need of a preliminary always-inlier pass. The reason is that the order of the traversal of the f

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

mtrofin updated this revision to Diff 306593. mtrofin added a comment. Herald added subscribers: wenlei, steven_wu. patched up tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91567/new/ https://reviews.llvm.org/D91567 Files: clang/test/CodeG

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

mtrofin added a comment. In D91567#2403544 , @aeubanks wrote: > In D91567#2403252 , @mtrofin wrote: > >> In D91567#2403236 , @aeubanks wrote: >> >>> One thing that would be n

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

mtrofin added a comment. In D91567#2403236 , @aeubanks wrote: > One thing that would be nice would be to have both inliners in the same CGSCC > pass manager to avoid doing SCC construction twice, but that would require > some shuffling of module/cgscc pa

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

mtrofin added a comment. In D91567#2403216 , @aeubanks wrote: >>> I'll run this through llvm-compile-time-tracker to see what the compile >>> time implications are. >> >> You mean for the variant where we ran some of the function passes, or you'd >> try

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

mtrofin added a comment. In D91567#2403173 , @aeubanks wrote: > In D91567#2400699 , @mtrofin wrote: > >> In D91567#2400207 , @aeubanks wrote: >> >>> What about removing the e

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

mtrofin updated this revision to Diff 306141. mtrofin added a comment. Running just the always inliner variant, without other passes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91567/new/ https://reviews.llvm.org/D91567 Files: clang/test/Fron

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

mtrofin added a comment. In D91567#2401021 , @dblaikie wrote: > In D91567#2398637 , @mtrofin wrote: > >> In D91567#2398623 , @dblaikie wrote: >> >>> In D91567#2398461

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

mtrofin added a comment. In D91567#2400207 , @aeubanks wrote: > What about removing the existing AlwaysInlinerPass and replacing it with this > one? Or is that something you were planning to do in a follow-up change? That's the plan, yes. >> open the op

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

mtrofin added a comment. In D91567#2398623 , @dblaikie wrote: > In D91567#2398461 , @mtrofin wrote: > >> In D91567#2398440 , @dblaikie wrote: >> Performing the mandatory

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

mtrofin added a comment. In D91567#2398440 , @dblaikie wrote: >> Performing the mandatory inlinings first simplifies the problem the full >> inliner needs to solve > > That confuses me a bit - is that suggesting that we don't run the > AlwaysInliner when

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

mtrofin added a comment. Please note: the patch isn't 100% ready, there are those tests that check how the pipeline is composed, which are unpleasant to fix, so I want to defer them to after we get agreement over the larger points this patch brings (i.e. pre-performing always inlinings, value i

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

mtrofin created this revision. mtrofin added reviewers: aeubanks, jdoerfert, davidxl, eraman. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added projects: clang, LLVM. mtrofin requested review of this revision. Enable performing mandatory inlinings upfront, by reusing the

[PATCH] D91229: [FileCheck] Flip -allow-unused-prefixes to false by default

mtrofin added a comment. In D91229#2389141 , @dblaikie wrote: >> because FileCheck won't accept more than one occurrence of >> --allow-unused-prefixes > > Perhaps this is a fixable issue? I think I was able to circumvent it in D91275

[PATCH] D91229: [FileCheck] Flip -allow-unused-prefixes to false by default

mtrofin added a comment. In D91229#2387923 , @jhenderson wrote: > Maybe one way to do this is to do what @RKSimon suggests in the D90281 > , and to enable it on a per-directory basis > using lit.local.cfg. That would pote

[PATCH] D91229: [FileCheck] Flip -allow-unused-prefixes to false by default

mtrofin created this revision. mtrofin added a reviewer: jhenderson. Herald added subscribers: llvm-commits, cfe-commits, frasercrmck, nikic, okura, jdoerfert, kuter, kerbowa, luismarques, apazos, sameer.abuasal, pzheng, pengfei, s.egerton, lenary, dmgreen, Jim, asbirlea, thopre, mstorsjo, jocewe

[PATCH] D91229: [FileCheck] Flip -allow-unused-prefixes to false by default

mtrofin added a comment. FYI - I realize the change is enormous. I don't necessarily mean to land it as-is, we can chunk it by directories and iteratively update this as those land. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91229/new/ https://

[PATCH] D90430: [clang][NFC] Remove unused FileCheck prefix

mtrofin accepted this revision. mtrofin added a comment. In D90430#2365397 , @google-llvm-upstream-contributions wrote: > lgtm ugh, sorry, that's an alias I am auto-logged in on my personal account. Still LGTM :) Repository: rG LLVM Github Monorepo

[PATCH] D90366: [ThinLTO] Fix empty .llvmcmd sections

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG13aee94bc710: [ThinLTO] Fix empty .llvmcmd sections (authored by mtrofin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D90366: [ThinLTO] Fix empty .llvmcmd sections

mtrofin updated this revision to Diff 301650. mtrofin marked an inline comment as done. mtrofin added a comment. updated description Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90366/new/ https://reviews.llvm.org/D90366 Files: clang/lib/Fronte

[PATCH] D90366: [ThinLTO] Strenghten the test for .llvmcmd embedding

mtrofin updated this revision to Diff 301641. mtrofin added a comment. fixed sentence Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90366/new/ https://reviews.llvm.org/D90366 Files: clang/lib/Frontend/CompilerInvocation.cpp clang/test/CodeGen/

[PATCH] D90366: [ThinLTO] Strenghten the test for .llvmcmd embedding

mtrofin added a comment. In D90366#2362052 , @tejohnson wrote: > The motivation and effects of the change are unclear to me. Does this mean > that the .llvmcmd section is always emitted? Or always emitted whenever any > bitcode embedding is requested? T

[PATCH] D90366: [ThinLTO] Strenghten the test for .llvmcmd embedding

mtrofin created this revision. mtrofin added a reviewer: tejohnson. Herald added subscribers: cfe-commits, steven_wu, hiraditya, inglorion. Herald added a reviewer: alexshap. Herald added a project: clang. mtrofin requested review of this revision. Always populate the CodeGenOptions::CmdArgs - the

  1   2   >