Page MenuHomePhabricator

modimo (Di Mo)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 7 2020, 3:53 PM (7 w, 2 d)

Recent Activity

Fri, Sep 25

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

Thu, Sep 17

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.).

Thu, Sep 17, 10:57 AM · Restricted Project

Wed, Sep 16

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.

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

Thanks @asbirlea!

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

Tue, Sep 15

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

Rebase #2

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

Mon, Sep 14

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

Remove unnecessary period

Mon, Sep 14, 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!

Mon, Sep 14, 3:11 PM · Restricted Project

Fri, Sep 11

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

Rebase

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

Thu, Sep 10

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

Wed, Sep 9

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.

Wed, Sep 9, 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.

Wed, Sep 9, 3:37 PM · Restricted Project
modimo added inline comments to D73739: Exception support for basic block sections.
Wed, Sep 9, 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?

Wed, Sep 9, 1:07 PM · Restricted Project

Thu, Sep 3

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.

Thu, Sep 3, 10:06 PM · Restricted Project

Wed, Sep 2

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

I think with this alongside the recently committed MachineFunctionSplitter (MFS) the MFS phase is a good place to split out EH landing pads in both profile and non-profile code. A small change in MFS will enable EH LP splits in profiling given they should always be cold. I think in non-profile mode a sub-flag can be added like -mfs-split-LP-only where EH LPs are always split out. Enabling a general static-analysis splitting could also be interesting but outside of EH LPs I'm not certain there's guaranteed gains in other scenarios.

Wed, Sep 2, 3:06 PM · Restricted Project

Tue, Sep 1

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

I'm rebased against 478eb98cd25cb0ebc01fc2c3889ae94d3f1797d3 and I'm seeing both added tests failing. They may need to be updated.

Tue, Sep 1, 4:31 PM · Restricted Project

Aug 28 2020

modimo retitled D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults from [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults to [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.
Aug 28 2020, 3:41 PM · Restricted Project, Restricted Project
modimo added a comment to D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.

I have no familiarity with BFI, so possibly stupid question: There is already some similar handling as part of BFIImpl here: https://github.com/llvm/llvm-project/blob/0f14b2e6cbb54c84ed3b00b0db521f5ce2d1e3f2/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h#L1043-L1058 What is the difference to that / why are both needed?

Aug 28 2020, 3:36 PM · Restricted Project, Restricted Project
modimo updated the diff for D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.

Remove redundant VH callback as @nikic helpfully pointed out!

Aug 28 2020, 3:36 PM · Restricted Project, Restricted Project

Aug 27 2020

modimo added inline comments to D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.
Aug 27 2020, 3:59 PM · Restricted Project, Restricted Project
modimo updated the diff for D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.

Condition usage of BFI to PGO in newPM as well

Aug 27 2020, 3:58 PM · Restricted Project, Restricted Project
modimo added a comment to D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.

As a general note, it may make sense to include BFI in the set of loop passes always preserved (getLoopPassPreservedAnalyses()), if its nature is to always be preserved (with some potential info loss) due to the callbacks deleting blocks. But since we're only looking at LICM effect for now, this can be a follow up when/if needed.

Aug 27 2020, 1:42 PM · Restricted Project, Restricted Project
modimo updated the summary of D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.
Aug 27 2020, 1:41 PM · Restricted Project, Restricted Project
modimo updated the diff for D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.

only use BFI when profile is enabled, have LICM mark BFI as preserved

Aug 27 2020, 1:36 PM · Restricted Project, Restricted Project

Aug 26 2020

modimo added inline comments to D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.
Aug 26 2020, 8:29 PM · Restricted Project, Restricted Project
modimo updated the diff for D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.

Remove usage need for BFI in LPM2 and set unswitching to preserve lazy BPI/BFI so it can remain in the same loop pass as LICM

Aug 26 2020, 8:29 PM · Restricted Project, Restricted Project
modimo added a comment to D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.

This change adds three PDT calculations to the standard pipeline. Please try to avoid the PDT calculations if PGO is not used, possibly by using LazyBPI.

Aug 26 2020, 3:57 PM · Restricted Project, Restricted Project
modimo updated the diff for D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.

Change to LazyBFI for legacy pass manager to prevent rebuilding the post-dominator tree

Aug 26 2020, 3:56 PM · Restricted Project, Restricted Project

Aug 21 2020

modimo added inline comments to D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.
Aug 21 2020, 8:02 PM · Restricted Project, Restricted Project
modimo updated the diff for D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.

@asbirlea Thanks for taking a look!

Aug 21 2020, 7:55 PM · Restricted Project, Restricted Project

Aug 18 2020

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

Commit my changes (crazy I know) so that the diff is actually updated for linting

Aug 18 2020, 1:31 PM · Restricted Project, Restricted Project
modimo updated the diff for D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.

Linting

Aug 18 2020, 1:29 PM · Restricted Project, Restricted Project
modimo requested review of D86156: [BFI] Make BFI information available through loop passes inside LoopStandardAnalysisResults.
Aug 18 2020, 10:40 AM · Restricted Project, Restricted Project

Aug 11 2020

modimo abandoned D85776: test 2.
Aug 11 2020, 1:07 PM · Restricted Project
modimo requested review of D85776: test 2.
Aug 11 2020, 1:05 PM · Restricted Project