Page MenuHomePhabricator

modimo (Di Mo)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 7 2020, 3:53 PM (44 w, 22 h)

Recent Activity

Yesterday

modimo updated the diff for D36850: [ThinLTO] Add norecurse function attribute propagation.

Clean up logic around bailing out. Separate out thinLTO statistics from general statistics. Run clang-format.

Fri, Jun 11, 7:18 PM · Restricted Project
modimo requested review of D104161: [NFC] [DwarfEHPrepare] Add additional stats for EH.
Fri, Jun 11, 5:07 PM · Restricted Project

Tue, Jun 8

modimo updated the diff for D36850: [ThinLTO] Add norecurse function attribute propagation.

@tejohnson
I was looking at the initial implementation and looking at purely the front entry of the SummaryList seems suspect given:

  1. There could be multiple entries from linkonce_odr
  2. The first entry isn't guaranteed to be a FunctionSummary
Tue, Jun 8, 12:01 AM · Restricted Project

Mon, Jun 7

modimo updated the diff for D36850: [ThinLTO] Add norecurse function attribute propagation.

Rebased, cleaned up and updated implementation. Also fixed up all the tests to be correct. Note that currently lld/COFF tests fail from LLD_IN_TEST=2 which runs tests in memory twice. I checked that this also fails from doing SCC traversal/update from synthetic function entry counts so it's not exclusive to my changes. I'm looking into this but sending this out in the mean time for review.

Mon, Jun 7, 11:46 PM · Restricted Project
modimo commandeered D36850: [ThinLTO] Add norecurse function attribute propagation.
Mon, Jun 7, 11:43 PM · Restricted Project
modimo added a comment to D102820: [Clang] Check for returns_nonnull when deciding to add allocation null checks.

Ping.

Mon, Jun 7, 11:39 PM · Restricted Project

Wed, Jun 2

modimo added a comment to D36850: [ThinLTO] Add norecurse function attribute propagation.

@tejohnson Do you know more about the status of this patch/@ncharlie? I'd like to help push this forward and can take over if they're no longer available.

The patch has been dormant since @ncharlie hasn't been active in LLVM for awhile, and no one else has had a chance to pick it up. Would be great if you would like to take it over and push it forward.

Wed, Jun 2, 6:13 PM · Restricted Project

Tue, Jun 1

modimo added a comment to D36850: [ThinLTO] Add norecurse function attribute propagation.

@tejohnson Do you know more about the status of this patch/@ncharlie? I'd like to help push this forward and can take over if they're no longer available.

Tue, Jun 1, 4:49 PM · Restricted Project

Wed, May 26

modimo added a comment to D102820: [Clang] Check for returns_nonnull when deciding to add allocation null checks.

@rsmith @lebedev.ri thoughts on adding this directly to FE generation? As mentioned this isn't strictly needed and the BE can elide the check but we can also not emit it to save on AST/IR processing cost.

Wed, May 26, 11:26 AM · Restricted Project

Thu, May 20

modimo added a comment to D102820: [Clang] Check for returns_nonnull when deciding to add allocation null checks.

Sounds reasonable to me! Can you double check whether this attribute gets correctly serialized/deserialized in face of CXXNewExpr? An example of how to test that would be in clang/test/PCH/cxx-method.cpp.

Thu, May 20, 7:55 PM · Restricted Project
modimo updated subscribers of D102820: [Clang] Check for returns_nonnull when deciding to add allocation null checks.

Discussing with @urnathan this makes more sense for the BE to handle when optimizing by eliding the generated null check. Confirmed this is indeed the case so removing the generation in the FE isn't really needed.

Thu, May 20, 1:45 PM · Restricted Project

Wed, May 19

modimo updated the diff for D102820: [Clang] Check for returns_nonnull when deciding to add allocation null checks.

Go back to single statement return, fix up ATTR_BUILTIN_NOTHROW_NEW test label that pointed to nothing to correct ATTR_NOBUILTIN_NOUNWIND_ALLOCSIZE label.

Wed, May 19, 6:32 PM · Restricted Project
modimo requested review of D102820: [Clang] Check for returns_nonnull when deciding to add allocation null checks.
Wed, May 19, 6:26 PM · Restricted Project

Apr 29 2021

modimo added a comment to D99487: [CodeGen] Port basic block sections from ELF to COFF.

Agree with @MaskRay I'd like to see a holistics plan/RFC on all the pieces that are needed to get this functionality. The functionality here is only tested when exceptions are actually propagated which is not common in LLVM test suite. You'll want to run and likely write very specific tests stressing the test matrix here of BBS + MFS + EH. Given all that, a larger plan that details how we can be confident that all the pieces have been accounted for and will be well tested is (IMO) imperative in getting this change reviewed and accepted.

Since this (the test matrix) is already done for ELF, would it be complete if the author added equivalent tests for COFF?

Apr 29 2021, 4:16 PM · Restricted Project

Apr 28 2021

modimo added a comment to D99487: [CodeGen] Port basic block sections from ELF to COFF.

Agree with @MaskRay I'd like to see a holistics plan/RFC on all the pieces that are needed to get this functionality. The functionality here is only tested when exceptions are actually propagated which is not common in LLVM test suite. You'll want to run and likely write very specific tests stressing the test matrix here of BBS + MFS + EH. Given all that, a larger plan that details how we can be confident that all the pieces have been accounted for and will be well tested is (IMO) imperative in getting this change reviewed and accepted.

Apr 28 2021, 9:13 PM · Restricted Project
modimo added a comment to D99487: [CodeGen] Port basic block sections from ELF to COFF.

Thanks for guiding MSVC's SEH tests! I found the SEH tests use cl (not clang-cl) to compile the c test file https://github.com/microsoft/compiler-tests/blob/2fc7859d212d5fb59628eb21aec50dfb4b934146/seh/runtests.cmd#L8, I'll try to change cl to clang-cl and make sure non-regressed.

Apr 28 2021, 9:12 PM · Restricted Project
modimo added a comment to D99487: [CodeGen] Port basic block sections from ELF to COFF.

For some added testing you can run MSVC's SEH tests to see that this works properly: https://github.com/microsoft/compiler-tests/tree/master/seh. I wouldn't be surprised if not all of these pass in the baseline but we certainly want to make sure the ones that are remain non-regressed.

Apr 28 2021, 1:18 PM · Restricted Project

Mar 25 2021

modimo added a comment to D96893: [OpenMP] libomp minor cleanup.

Thanks!

Mar 25 2021, 1:58 PM · Restricted Project

Mar 24 2021

modimo added a comment to D96893: [OpenMP] libomp minor cleanup.

@protze.joachim Should we raise this issue up somewhere else for discussion? Bug report/mailing list?

Mar 24 2021, 12:45 PM · Restricted Project

Mar 17 2021

modimo added a comment to D96893: [OpenMP] libomp minor cleanup.

That makes sense, thanks for the explanation and additional investigation @protze.joachim!

Mar 17 2021, 11:52 AM · Restricted Project

Mar 1 2021

modimo added a comment to D96893: [OpenMP] libomp minor cleanup.

Hi @AndreyChurbanov I'm seeing a test failure (openmp/runtime/test/ompt/tasks/task_if0-depend.c) with this change under a -DCMAKE_BUILD_TYPE=debug build setup with our internal branch based off of the newly added KMP_DEBUG_ASSERT(n >= 0);. This doesn't seem to reproduce directly under trunk but looking into it I think the bug is optimization dependent but still present.

Mar 1 2021, 11:35 PM · Restricted Project

Jan 25 2021

modimo committed rGce7f9cdb50a9: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from… (authored by modimo).
[InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from…
Jan 25 2021, 3:39 PM
modimo closed D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks.
Jan 25 2021, 3:39 PM · Restricted Project

Jan 22 2021

modimo updated the diff for D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks.

Restrict replay to default advisor only

Jan 22 2021, 2:21 PM · Restricted Project
modimo added inline comments to D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks.
Jan 22 2021, 1:42 PM · Restricted Project
modimo updated the diff for D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks.

Nest original advisors in ReplayInlineAdvisor rather than the other way around.

Jan 22 2021, 1:36 PM · Restricted Project

Jan 21 2021

modimo updated the diff for D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks.

Wrap ReplayInlineAdvisor into InlineAdvisor so that we can fall back to the original Advisor if we don't want to follow the replay. I think the composition of advisors here makes sense but I'm not sure so I'm very open to different approaches.

Jan 21 2021, 3:54 PM · Restricted Project
modimo added inline comments to D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it.
Jan 21 2021, 3:11 PM · Restricted Project, Restricted Project

Jan 15 2021

modimo added inline comments to D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks.
Jan 15 2021, 11:56 AM · Restricted Project

Jan 14 2021

modimo updated the diff for D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks.

Add DEFAULT testing to make sure baseline inlining differs from replay. Fix copy-paste error in flag description for -cgscc-inline-replay

Jan 14 2021, 3:20 PM · Restricted Project
modimo added inline comments to D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks.
Jan 14 2021, 3:15 PM · Restricted Project
modimo updated the summary of D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks.
Jan 14 2021, 3:00 PM · Restricted Project
modimo added inline comments to D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it.
Jan 14 2021, 2:54 PM · Restricted Project, Restricted Project

Jan 12 2021

modimo committed rG2a49b7c64a33: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it (authored by modimo).
[Inliner] Change inline remark format and update ReplayInlineAdvisor to use it
Jan 12 2021, 1:44 PM
modimo closed D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it.
Jan 12 2021, 1:44 PM · Restricted Project, Restricted Project

Jan 11 2021

modimo updated the diff for D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it.

Add comment about best effort approach of replay.

Jan 11 2021, 7:47 PM · Restricted Project, Restricted Project
modimo added a comment to D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it.

I wouldn't block this patch on that - we can do the refactoring subsequently.

Jan 11 2021, 7:46 PM · Restricted Project, Restricted Project
modimo added a comment to D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it.

The modifications to the DefaultInlineAdvice is an attempt to solve the following problem:

  1. In the SampleProfile inliner it emits remarks through the "legacy" interface with emitInlinedInto. For replay if advice generates remarks you'll get duplicate remarks but if it doesn't you get a single set (which is what's happening today and correct).
  2. In the CGSCC inliner in newPM it emits remarks through InlineAdvice via recordInlining and nowhere else. For replay if InlineAdvice generates remarks you're good but if it doesn't you get no remarks.
Jan 11 2021, 12:59 PM · Restricted Project, Restricted Project

Jan 8 2021

modimo added inline comments to D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks.
Jan 8 2021, 6:29 PM · Restricted Project
modimo updated the summary of D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks.
Jan 8 2021, 6:15 PM · Restricted Project
modimo updated the diff for D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks.

Move the ReplayInlineAdvisor.cpp/h and SampleProfile.cpp files to D94333 as they need to be atomic with the remarks format change.

Jan 8 2021, 5:45 PM · Restricted Project
modimo retitled D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it from [Inliner] Change inline remark format and update ReplayInlineAdvisor to it to [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it.
Jan 8 2021, 5:35 PM · Restricted Project, Restricted Project
modimo retitled D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it from [Inliner] Change inline remark format to [Inliner] Change inline remark format and update ReplayInlineAdvisor to it.
Jan 8 2021, 5:27 PM · Restricted Project, Restricted Project
modimo added a comment to D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it.

The proposal in the patch is to use line:col.discriminator (which is reasonable), but the implementation uses line:col:discriminator -- please update the patch to be consistent.

Jan 8 2021, 5:25 PM · Restricted Project, Restricted Project
modimo updated the diff for D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it.

Split changes with D94334 better: now the ReplayInlineAdvisor update which needs to be atomic with the formatting change is moved over to here. Removed ReplayInlineAdvice and modified DefaultInlineAdvice to provide the desired functionality. Fixed up tests.

Jan 8 2021, 4:58 PM · Restricted Project, Restricted Project
modimo added a reviewer for D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it: davidxl.
Jan 8 2021, 1:39 PM · Restricted Project, Restricted Project
modimo added a reviewer for D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks: davidxl.
Jan 8 2021, 1:39 PM · Restricted Project
modimo retitled D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks from cgscc replay to [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks.
Jan 8 2021, 1:39 PM · Restricted Project
modimo retitled D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it from change remark format to [Inliner] Change inline remark format.
Jan 8 2021, 12:41 PM · Restricted Project, Restricted Project
modimo requested review of D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks.
Jan 8 2021, 12:36 PM · Restricted Project
modimo requested review of D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it.
Jan 8 2021, 12:35 PM · Restricted Project, Restricted Project
modimo abandoned D94331: Initial.
Jan 8 2021, 12:27 PM · Restricted Project
modimo requested review of D94331: Initial.
Jan 8 2021, 12:27 PM · Restricted Project

Dec 3 2020

modimo abandoned D92592: Fix layering issue by moving AliasScopeNode from ScopedNoAliasAA.h to Metadata.h.

Fixed by https://reviews.llvm.org/rG756fa8b9be0ca3458073952495c323ba9d04036d

Dec 3 2020, 11:06 AM · Restricted Project
modimo added a comment to D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.

Appreciate the quick commit! Guess I gotta be faster with D92592 :D.

Dec 3 2020, 11:05 AM · Restricted Project
modimo added a reviewer for D92592: Fix layering issue by moving AliasScopeNode from ScopedNoAliasAA.h to Metadata.h: MaskRay.
Dec 3 2020, 11:03 AM · Restricted Project
modimo requested review of D92592: Fix layering issue by moving AliasScopeNode from ScopedNoAliasAA.h to Metadata.h.
Dec 3 2020, 11:02 AM · Restricted Project
modimo added a comment to D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.

Good catch @MaskRay. If I'm understanding correctly I think the correct approach is to move the class AliasScopeNode I need to metadata.h to fix this?

Dec 3 2020, 10:44 AM · Restricted Project
modimo added a comment to D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.

Certainly, I appreciate the thorough and quick review! Best of luck with your restrict patches!

Dec 3 2020, 9:29 AM · Restricted Project
modimo committed rG18603319321a: [MemCpyOpt] Correctly merge alias scopes during call slot optimization (authored by modimo).
[MemCpyOpt] Correctly merge alias scopes during call slot optimization
Dec 3 2020, 9:24 AM
modimo closed D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.
Dec 3 2020, 9:24 AM · Restricted Project

Dec 2 2020

modimo committed rGc1ba991e8dd6: [NFC] Fix typo (authored by modimo).
[NFC] Fix typo
Dec 2 2020, 10:24 PM
modimo updated the diff for D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.

Updating new merge test to explicitly look for the correct merged scopes. Thanks @jeroen.dobbelaere!

Dec 2 2020, 1:47 PM · Restricted Project

Dec 1 2020

modimo added inline comments to D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.
Dec 1 2020, 3:27 PM · Restricted Project
modimo updated the diff for D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.

Go with intersection of domains then union the scopes within those domains as discussed. Updating tests to match latest behavior.

Dec 1 2020, 3:21 PM · Restricted Project

Nov 30 2020

modimo added inline comments to D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.
Nov 30 2020, 1:03 PM · Restricted Project
modimo updated the diff for D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.

Move the fix to getMostGenericAliasScope. Renamed metadata in test case.

Nov 30 2020, 1:01 PM · Restricted Project

Nov 19 2020

modimo added a comment to D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.

@jeroen.dobbelaere I think the correct merging in all cases requires this strategy (or one which checks that all the domains match). If true (@hfinkel?) that'll be a larger change that needs more testing.

Nov 19 2020, 3:27 PM · Restricted Project
modimo retitled D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization from [MemCpyOpt] Bail on call slot optimization if it merges alias scopes to [MemCpyOpt] Correctly merge alias scopes during call slot optimization.
Nov 19 2020, 3:23 PM · Restricted Project
modimo updated the diff for D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.

Changing to conservative merge of metadata rather than bailing on optimization. Updated test case due to that.

Nov 19 2020, 3:21 PM · Restricted Project

Nov 18 2020

modimo added inline comments to D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.
Nov 18 2020, 6:23 PM · Restricted Project
modimo updated the summary of D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.
Nov 18 2020, 6:19 PM · Restricted Project
modimo updated the summary of D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.
Nov 18 2020, 6:16 PM · Restricted Project
modimo updated the diff for D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.

Update unit test with correct set of metadata

Nov 18 2020, 6:11 PM · Restricted Project
modimo added a comment to D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.

Hi Modi,

Following line in your input example is wrong and explains why the resulting alias info is corrupt:

call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %tmp, i8* align 8 %src, i64 1, i1 false), !alias.scope !1

It should be !alias.scope !0

Thanks for verifying! Good catch, I'll update the patch with this fixed up example.

Nov 18 2020, 5:55 PM · Restricted Project
modimo added a comment to D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.

I've looked more into what alias.scopes are and I think I've got a better handle on what's actually wrong. In the meantime let me try to describe the original problem better. The first section is checking my understanding and making sure we're on the same page. Skip to the second part for the bug details.

My understanding of domains

Let's say we have the following (I slightly modified test/Transforms/Inline/noalias.ll for this):

Nov 18 2020, 2:44 PM · Restricted Project

Nov 16 2020

modimo updated the summary of D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.
Nov 16 2020, 3:46 PM · Restricted Project
modimo updated the diff for D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.

Add more comments in test case

Nov 16 2020, 3:46 PM · Restricted Project
modimo updated the summary of D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.
Nov 16 2020, 3:46 PM · Restricted Project
modimo requested review of D91576: [MemCpyOpt] Correctly merge alias scopes during call slot optimization.
Nov 16 2020, 3:44 PM · Restricted Project

Oct 12 2020

modimo abandoned D89176: Modify split CallSiteRanges to use correct lengths .

@MaskRay Ah the action table start is inferred as the end of the CST. Saves a pointer but shackles the implementation. These tables will be fairly small (effectively 1 entry per "catch" in a function) and not all CST would need a corresponding action table (only those inside "try" do) so duplicating them is not as far-fetched as it would initially sound.

Oct 12 2020, 12:42 PM · Restricted Project

Oct 9 2020

modimo updated the summary of D89176: Modify split CallSiteRanges to use correct lengths .
Oct 9 2020, 6:49 PM · Restricted Project
modimo updated the diff for D89176: Modify split CallSiteRanges to use correct lengths .

Remove useless "if"

Oct 9 2020, 6:48 PM · Restricted Project
modimo requested review of D89176: Modify split CallSiteRanges to use correct lengths .
Oct 9 2020, 6:46 PM · Restricted Project

Oct 1 2020

modimo added a comment to D73739: Exception support for basic block sections.

Glad to see this landing! I'm testing locally with my change to enable this purely for landing blocks.

Oct 1 2020, 4:16 PM · Restricted Project

Sep 25 2020

modimo added inline comments to D73739: Exception support for basic block sections.
Sep 25 2020, 5:32 PM · Restricted Project

Sep 17 2020

modimo added a comment to D73739: Exception support for basic block sections.

Although I can definitely work with you to diagnose any potential issues on ARM64, I suggest we don't write tests for this patch before the base feature is well-tested.
(On related note, a recent commit (rG157cd93b48a9) limited -fbasic-block-sections to X86.).

Sep 17 2020, 10:57 AM · Restricted Project

Sep 16 2020

modimo added a comment to D73739: Exception support for basic block sections.

@rahmanl Have you had a chance to run your added test on ARM64 as another itanium C++ ABI target to make sure it works properly? Having testing there is fine as a follow-up but wanted to make sure we're in agreement here. Otherwise changes LGTM, please let @MaskRay have a chance to provide any additional feedback.

Sep 16 2020, 5:54 PM · Restricted Project
modimo added a comment to D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.

Thanks @asbirlea!

Sep 16 2020, 1:54 PM · Restricted Project, Restricted Project

Sep 15 2020

modimo updated the diff for D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.

Rebase #2

Sep 15 2020, 4:07 PM · Restricted Project, Restricted Project

Sep 14 2020

modimo updated the diff for D87551: [LICM] Make Loop ICM profile aware again.

Remove unnecessary period

Sep 14 2020, 3:15 PM · Restricted Project
modimo updated the diff for D87551: [LICM] Make Loop ICM profile aware again.

Fix merge issues with comment, updated description to include benefit we see from this change. Thanks @asbirlea for the quick review!

Sep 14 2020, 3:11 PM · Restricted Project

Sep 11 2020

modimo updated the diff for D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.

Rebase

Sep 11 2020, 4:10 PM · Restricted Project, Restricted Project
modimo requested review of D87551: [LICM] Make Loop ICM profile aware again.
Sep 11 2020, 3:33 PM · Restricted Project

Sep 10 2020

modimo added inline comments to D73739: Exception support for basic block sections.
Sep 10 2020, 5:33 PM · Restricted Project

Sep 9 2020

modimo added a comment to D73739: Exception support for basic block sections.

These changes will also apply to other Itanium C++ ABI targets (arm32/arm64/RISCV etc.) so adding testing for at least another target is good. Trying this out for arm64 hits an ICE here:

case TargetOpcode::EH_LABEL:
  if (MBB.isBeginSection() && MBB.isEHPad() &&
      ((*std::prev(MI.getIterator())).getOpcode() ==
       TargetOpcode::CFI_INSTRUCTION)) {

which is worth following up on.

Sep 9 2020, 3:55 PM · Restricted Project
modimo added a comment to D73739: Exception support for basic block sections.

Taking a closer look at the test (haven't delved into the code yet): the PIC vs non-PIC difference of .gcc_exception_table looks strange.
.gcc_except_table (please also test its section flags "a") has an absolute relocation referencing main.2. This is not good. In all targets (except RISC-V -mno-relax), .gcc_except_table is a relocation-free section.

Sep 9 2020, 3:37 PM · Restricted Project
modimo added inline comments to D73739: Exception support for basic block sections.
Sep 9 2020, 3:26 PM · Restricted Project
modimo added a comment to D73739: Exception support for basic block sections.

Once this patch is in we can look into splitting ehpads out though I'm more inclined to enhance the static profile count mechanism to account for ehpads appropriately rather than adding a new flag to MFS.

On this front, is there a specific reviewer that we're waiting for here to get this change through?

Sep 9 2020, 1:07 PM · Restricted Project

Sep 3 2020

modimo added a comment to D86859: [Coroutine] Make dealing with alloca spills more robust.

Looking at https://bugs.llvm.org/show_bug.cgi?id=36578:
Andrew Kelley 2019-08-11 09:00:59 PDT
I'm no longer subscribed to this bug report. I've come to the conclusion that LLVM's coroutine API is not worth using, and resorting to implementing coroutines directly in the frontend.

Sep 3 2020, 10:06 PM · Restricted Project