Page MenuHomePhabricator

aeubanks (Arthur Eubanks)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 12 2020, 11:23 AM (61 w, 6 d)

Recent Activity

Today

aeubanks committed rG326da4adcb8d: [FuncAttrs] Always preserve FunctionAnalysisManagerCGSCCProxy (authored by aeubanks).
[FuncAttrs] Always preserve FunctionAnalysisManagerCGSCCProxy
Tue, Apr 20, 4:38 PM
aeubanks closed D100893: [FuncAttrs] Always preserve FunctionAnalysisManagerCGSCCProxy.
Tue, Apr 20, 4:38 PM · Restricted Project
aeubanks requested review of D100912: [docs][NewPM] Add section on analyses.
Tue, Apr 20, 4:36 PM · Restricted Project
aeubanks added a comment to D100893: [FuncAttrs] Always preserve FunctionAnalysisManagerCGSCCProxy.

This shouldn't help compile times yet since we still end up invalidating all the same analyses in FunctionAnalysisManagerCGSCCProxy::Result::invalidate, but it's a step in that direction.
I want to look into preserving analyses for functions not modified in a CGSCC pass, even if other functions are modified, to help D98103 and compile times in general.

Tue, Apr 20, 3:13 PM · Restricted Project
aeubanks requested review of D100893: [FuncAttrs] Always preserve FunctionAnalysisManagerCGSCCProxy.
Tue, Apr 20, 3:08 PM · Restricted Project
aeubanks updated the summary of D99629: [GlobalOpt] Don't replace alias with aliasee if aliasee is interposable.
Tue, Apr 20, 1:34 PM · Restricted Project
aeubanks updated the diff for D99629: [GlobalOpt] Don't replace alias with aliasee if aliasee is interposable.

keep check of alias itself

Tue, Apr 20, 1:33 PM · Restricted Project
aeubanks added a comment to D99774: [LoopUtils] Populate sibling loops in reverse program order on new pass manager.

I think we should stick with one way unless there's specific need otherwise. This patch overall is fine, multiple people have said that we should just stick with the legacy order unless somebody has a better idea.

Tue, Apr 20, 10:50 AM · Restricted Project
aeubanks committed rG5e71b9fa933a: Explicitly pass type to cast load constant folding result (authored by aeubanks).
Explicitly pass type to cast load constant folding result
Tue, Apr 20, 12:59 AM
aeubanks closed D100718: Explicitly pass type to cast load constant folding result.
Tue, Apr 20, 12:59 AM · Restricted Project
aeubanks added a comment to D100718: Explicitly pass type to cast load constant folding result.

I've updated the commit message to mention that.

Tue, Apr 20, 12:58 AM · Restricted Project

Yesterday

aeubanks added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

another false positive:

Mon, Apr 19, 12:52 PM · Restricted Project
aeubanks added a comment to D100718: Explicitly pass type to cast load constant folding result.

I'm not 100% sure this is NFC, I think it's possible this might cause some behavior change with a very specific set of bitcasts, although it would be hard to reproduce since passes typically constant fold everything, not just one specific load instruction. But I built chrome with and without this change and it's the exact same size.

Mon, Apr 19, 11:09 AM · Restricted Project
aeubanks committed rG5561b48b7072: [test] Make global in split-gep-and-gvn.ll not constant (authored by aeubanks).
[test] Make global in split-gep-and-gvn.ll not constant
Mon, Apr 19, 11:04 AM

Sun, Apr 18

aeubanks added reviewers for D100718: Explicitly pass type to cast load constant folding result: dblaikie, rnk.
Sun, Apr 18, 11:21 PM · Restricted Project
aeubanks requested review of D100718: Explicitly pass type to cast load constant folding result.
Sun, Apr 18, 1:43 AM · Restricted Project

Fri, Apr 16

aeubanks added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

-Wunused-but-set-variable is firing on x at [1]

Fri, Apr 16, 10:53 AM · Restricted Project
aeubanks added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

Running this over Chromium, I'm getting

$ cat /tmp/a.cc
struct A {
  int i;
  A(int i): i(i) {}
};
Fri, Apr 16, 10:44 AM · Restricted Project

Thu, Apr 15

aeubanks committed rG9c776c2fa2bd: [NFC][NewPM] Remove some AnalysisManager invalidate methods (authored by aeubanks).
[NFC][NewPM] Remove some AnalysisManager invalidate methods
Thu, Apr 15, 4:57 PM
aeubanks closed D100519: [NFC][NewPM] Remove some AnalysisManager invalidate methods.
Thu, Apr 15, 4:57 PM · Restricted Project
aeubanks added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

the description should be clearer about what these warnings do

Thu, Apr 15, 11:44 AM · Restricted Project
aeubanks committed rGc8f0a7c215ab: [NewPM] Cleanup IR printing instrumentation (authored by aeubanks).
[NewPM] Cleanup IR printing instrumentation
Thu, Apr 15, 9:51 AM
aeubanks closed D100231: [NewPM] Cleanup IR printing instrumentation.
Thu, Apr 15, 9:51 AM · Restricted Project, Restricted Project

Wed, Apr 14

aeubanks added a comment to D100519: [NFC][NewPM] Remove some AnalysisManager invalidate methods.

Ah yeah profile summary info could be large.

Wed, Apr 14, 8:53 PM · Restricted Project
aeubanks added inline comments to D100519: [NFC][NewPM] Remove some AnalysisManager invalidate methods.
Wed, Apr 14, 8:04 PM · Restricted Project
aeubanks added a comment to D98103: [NPM] Do not run function simplification pipeline unnecessarily.

Talking offline with mtrofin, IMO in an ideal world:

  • the analysis should be invalidatable like a typical stateful analysis result (I hadn't noticed the analysis here was immutable, that's why I was confused, plus the invalidate vs clear distinction)
  • the logic around InvalidateSA shouldn't be necessary, the existing invalidation infra should handle it
  • all CGSCC passes specify that they preserve all function analyses and only manually invalidate function analyses on functions that they have modified
    • this wouldn't happen in updateCGAndAnalysisManagerForPass() since that's mostly just for keeping the call graph up to date, passes that don't change the call graph won't call that (e.g. PostOrderFunctionAttrsPass)
    • this could save some compile time in general since right now we're invalidating function analysis results for all functions in an SCC if any function has changed
Wed, Apr 14, 6:54 PM · Restricted Project
aeubanks added a comment to D100519: [NFC][NewPM] Remove some AnalysisManager invalidate methods.

This does break https://reviews.llvm.org/D98103, but IMO that analysis shouldn't be immutable, but rather just a typical function analysis that can be invalidated like any other stateful function analysis result.

Wed, Apr 14, 6:45 PM · Restricted Project
aeubanks requested review of D100519: [NFC][NewPM] Remove some AnalysisManager invalidate methods.
Wed, Apr 14, 6:43 PM · Restricted Project
aeubanks added a comment to D100231: [NewPM] Cleanup IR printing instrumentation.

I'm not sure that having a helper method with 4 lambdas is any cleaner. Normal control flow in a function is easier to read and understand, and not really that much more code.

Wed, Apr 14, 3:16 PM · Restricted Project, Restricted Project
aeubanks updated the diff for D100231: [NewPM] Cleanup IR printing instrumentation.

address comments

Wed, Apr 14, 3:14 PM · Restricted Project, Restricted Project
aeubanks added inline comments to D98103: [NPM] Do not run function simplification pipeline unnecessarily.
Wed, Apr 14, 1:47 PM · Restricted Project

Mon, Apr 12

aeubanks added a comment to rGf2e4f3eff3c9: Reapply "[DebugInfo] Use variadic debug values to salvage BinOps and GEP instrs….

Ah sorry I came up with a repro but didn't find this phab review earlier. https://crbug.com/1198356#c1

Mon, Apr 12, 10:54 PM
aeubanks removed a reviewer for D90285: [CallGraphUpdater][Attributor][FIX] Allow standalone function replacement: aeubanks.
Mon, Apr 12, 5:03 PM · Restricted Project
aeubanks abandoned D97816: [clang] Note that -debug-pass only works with legacy PM when using new PM.
Mon, Apr 12, 5:02 PM · Restricted Project
aeubanks added a comment to D86657: Add new hidden option -print-crash-IR that prints out IR that caused opt pipeline to crash.

maybe an assert(false) is more portable?

Mon, Apr 12, 4:57 PM · Restricted Project
aeubanks added a comment to D98103: [NPM] Do not run function simplification pipeline unnecessarily.

some comments on the summary

  • we should clarify that this is under a flag and is not currently changing any defaults. [2] says that, but we should make sure it's very clear somewhere besides a footer
  • the simplification pipeline is known to not be idempotent (and it would be very hard to make it so without majorly regressing compile times), so should not have a code quality effect is not true at all. but we should note that in a perfect world this wouldn't change code quality if the function simplification were perfect
  • the function passes will be re-run -> the function simplification pipeline will be re-run is a bit clearer, same with This patch allows the function passes be skipped if they were run once and a function was not modified since.
Mon, Apr 12, 4:35 PM · Restricted Project
aeubanks committed rGa8ab1f98d22c: [Evaluator] Look through invariant.group intrinsics (authored by aeubanks).
[Evaluator] Look through invariant.group intrinsics
Mon, Apr 12, 4:12 PM
aeubanks closed D98843: [Evaluator] Look through invariant.group intrinsics.
Mon, Apr 12, 4:12 PM · Restricted Project
aeubanks added a reviewer for D100231: [NewPM] Cleanup IR printing instrumentation: jamieschmeiser.
Mon, Apr 12, 3:07 PM · Restricted Project, Restricted Project
aeubanks planned changes to D99240: [ConstantFolding] Look through local aliases when simplify GEPs.

I'll see if I can do this just for load instructions

Mon, Apr 12, 3:04 PM · Restricted Project
aeubanks updated the diff for D100231: [NewPM] Cleanup IR printing instrumentation.

test fixes

Mon, Apr 12, 3:03 PM · Restricted Project, Restricted Project
aeubanks updated the diff for D100231: [NewPM] Cleanup IR printing instrumentation.

more changes

Mon, Apr 12, 1:58 PM · Restricted Project, Restricted Project
aeubanks updated the summary of D100231: [NewPM] Cleanup IR printing instrumentation.
Mon, Apr 12, 1:44 PM · Restricted Project, Restricted Project
aeubanks updated the diff for D100231: [NewPM] Cleanup IR printing instrumentation.

rebase, major overhaul

Mon, Apr 12, 1:43 PM · Restricted Project, Restricted Project
aeubanks committed rGbe00edfee55e: [NewPM] Fix -print-changed when a -filter-print-funcs function is removed (authored by aeubanks).
[NewPM] Fix -print-changed when a -filter-print-funcs function is removed
Mon, Apr 12, 12:01 PM
aeubanks closed D100237: [NewPM] Fix -print-changed when a -filter-print-funcs function is removed.
Mon, Apr 12, 12:01 PM · Restricted Project
aeubanks updated the diff for D100237: [NewPM] Fix -print-changed when a -filter-print-funcs function is removed.

print "IR Deleted"

Mon, Apr 12, 11:43 AM · Restricted Project
aeubanks added a comment to D98103: [NPM] Do not run function simplification pipeline unnecessarily.

nit with the summary, I think of "[NFC]" as more of refactoring types of changes, this adds new functionality under a flag

Mon, Apr 12, 11:07 AM · Restricted Project
aeubanks committed rG269b335bd733: [Inliner] Propagate SROA analysis through invariant group intrinsics (authored by aeubanks).
[Inliner] Propagate SROA analysis through invariant group intrinsics
Mon, Apr 12, 10:55 AM
aeubanks closed D100249: [Inliner] Propagate SROA analysis through invariant group intrinsics.
Mon, Apr 12, 10:55 AM · Restricted Project
aeubanks updated the diff for D100249: [Inliner] Propagate SROA analysis through invariant group intrinsics.

2>&1

Mon, Apr 12, 10:54 AM · Restricted Project
aeubanks added a comment to D100249: [Inliner] Propagate SROA analysis through invariant group intrinsics.

The paranoid in me would argue for having a flag enabling this, should there be any regressions, but if we feel this is benign, ok.

Mon, Apr 12, 10:45 AM · Restricted Project

Sat, Apr 10

aeubanks added a reverting change for rG6210261ecb21: Remove "Rewrite Symbols" from codegen pipeline: rGc88b87f9ce64: Revert "Remove "Rewrite Symbols" from codegen pipeline".
Sat, Apr 10, 11:29 PM
aeubanks committed rGc88b87f9ce64: Revert "Remove "Rewrite Symbols" from codegen pipeline" (authored by aeubanks).
Revert "Remove "Rewrite Symbols" from codegen pipeline"
Sat, Apr 10, 11:29 PM
aeubanks added a reverting change for D99707: Remove "Rewrite Symbols" from codegen pipeline: rGc88b87f9ce64: Revert "Remove "Rewrite Symbols" from codegen pipeline".
Sat, Apr 10, 11:29 PM · Restricted Project
aeubanks committed rG6210261ecb21: Remove "Rewrite Symbols" from codegen pipeline (authored by aeubanks).
Remove "Rewrite Symbols" from codegen pipeline
Sat, Apr 10, 10:56 PM
aeubanks closed D99707: Remove "Rewrite Symbols" from codegen pipeline.
Sat, Apr 10, 10:56 PM · Restricted Project
aeubanks requested review of D100249: [Inliner] Propagate SROA analysis through invariant group intrinsics.
Sat, Apr 10, 12:02 PM · Restricted Project

Fri, Apr 9

aeubanks added a reviewer for D100237: [NewPM] Fix -print-changed when a -filter-print-funcs function is removed: jamieschmeiser.
Fri, Apr 9, 4:11 PM · Restricted Project
aeubanks requested review of D100237: [NewPM] Fix -print-changed when a -filter-print-funcs function is removed.
Fri, Apr 9, 4:10 PM · Restricted Project
aeubanks requested review of D100231: [NewPM] Cleanup IR printing instrumentation.
Fri, Apr 9, 2:22 PM · Restricted Project, Restricted Project
aeubanks accepted D100116: Reuse temporary files for print-changed=diff..

I think if we're removing the file before doing another diff there shouldn't be any race conditions. But anyway this is fine.

Fri, Apr 9, 10:43 AM · Restricted Project
aeubanks added a comment to D100116: Reuse temporary files for print-changed=diff..

Can we delete the files? That would fix this and also would fix us leaving around files after every invocation of opt

Fri, Apr 9, 10:10 AM · Restricted Project

Thu, Apr 8

aeubanks committed rG4c89bcadf6ca: [LICM] Hoist loads with invariant.group metadata (authored by aeubanks).
[LICM] Hoist loads with invariant.group metadata
Thu, Apr 8, 9:58 PM
aeubanks closed D99784: [LICM] Hoist loads with invariant.group metadata.
Thu, Apr 8, 9:57 PM · Restricted Project
aeubanks added a comment to D98843: [Evaluator] Look through invariant.group intrinsics.

ping

Thu, Apr 8, 2:23 PM · Restricted Project
aeubanks committed rGc5d1ccbcdfb1: [GVN] Properly invalidate ICF cache when we simplify a value (authored by aeubanks).
[GVN] Properly invalidate ICF cache when we simplify a value
Thu, Apr 8, 2:02 PM
aeubanks closed D99977: [GVN] Properly invalidate ICF cache when we simplify a value.
Thu, Apr 8, 2:02 PM · Restricted Project
aeubanks updated the diff for D99977: [GVN] Properly invalidate ICF cache when we simplify a value.

address comment

Thu, Apr 8, 1:16 PM · Restricted Project
aeubanks updated the diff for D99977: [GVN] Properly invalidate ICF cache when we simplify a value.

simplify test more
move cache invalidation into ICF method

Thu, Apr 8, 12:40 PM · Restricted Project
aeubanks retitled D99977: [GVN] Properly invalidate ICF cache when we simplify a value from [GVN] Clear ICF cache when we simplify a value to [GVN] Properly invalidate ICF cache when we simplify a value.
Thu, Apr 8, 11:52 AM · Restricted Project
aeubanks updated the diff for D99977: [GVN] Properly invalidate ICF cache when we simplify a value.

selectively invalidate

Thu, Apr 8, 11:51 AM · Restricted Project
aeubanks added a comment to D100116: Reuse temporary files for print-changed=diff..

I think I made the request because I thought that FileName and FD were being populated every time, but that's actually not the case.

Thu, Apr 8, 10:58 AM · Restricted Project

Wed, Apr 7

aeubanks updated the diff for D99977: [GVN] Properly invalidate ICF cache when we simplify a value.

actually make the test do something

Wed, Apr 7, 11:43 PM · Restricted Project
aeubanks added reviewers for D99977: [GVN] Properly invalidate ICF cache when we simplify a value: rnk, hans.
Wed, Apr 7, 11:10 PM · Restricted Project
aeubanks added a comment to D100079: [Attributor] Don't modify functions outside of the current SCC.

I'm not actually sure that this is the right change, it was more of a "does something like this make sense"

Wed, Apr 7, 6:02 PM · Restricted Project
aeubanks requested review of D100079: [Attributor] Don't modify functions outside of the current SCC.
Wed, Apr 7, 5:09 PM · Restricted Project
aeubanks updated the diff for D99707: Remove "Rewrite Symbols" from codegen pipeline.

rebase after revert

Wed, Apr 7, 11:48 AM · Restricted Project
aeubanks committed rG90af13447333: Revert "[AsmPrinter] Delete dead takeDeletedSymbsForFunction()" (authored by aeubanks).
Revert "[AsmPrinter] Delete dead takeDeletedSymbsForFunction()"
Wed, Apr 7, 11:47 AM
aeubanks added a reverting change for rG9583a3f26258: [AsmPrinter] Delete dead takeDeletedSymbsForFunction(): rG90af13447333: Revert "[AsmPrinter] Delete dead takeDeletedSymbsForFunction()".
Wed, Apr 7, 11:47 AM
aeubanks added a comment to D99774: [LoopUtils] Populate sibling loops in reverse program order on new pass manager.

Traversing forward for the loops can cause a value in an earlier loop to be evaluated, which could help a later loop, right? I'm now not sure that it's such a clear tradeoff.
For example, scev-expander-preserve-lcssa.ll looks worse now.

Wed, Apr 7, 11:14 AM · Restricted Project
aeubanks abandoned D97814: [CGSCC] Skip SCC if we will revisit it later.
Wed, Apr 7, 9:54 AM · Restricted Project

Tue, Apr 6

aeubanks added a comment to D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1.

could you clang-format the patch?

Tue, Apr 6, 11:24 PM · Restricted Project, Restricted Project
aeubanks updated the summary of D99240: [ConstantFolding] Look through local aliases when simplify GEPs.
Tue, Apr 6, 6:45 PM · Restricted Project
aeubanks updated the diff for D99240: [ConstantFolding] Look through local aliases when simplify GEPs.

only consider non-interposable aliases with non-interposable aliasees

Tue, Apr 6, 6:45 PM · Restricted Project
aeubanks added a comment to D99629: [GlobalOpt] Don't replace alias with aliasee if aliasee is interposable.

I can keep the check to see if the alias itself is interposable, although it sounds like that shouldn't matter

Tue, Apr 6, 2:12 PM · Restricted Project
aeubanks requested review of D99977: [GVN] Properly invalidate ICF cache when we simplify a value.
Tue, Apr 6, 10:52 AM · Restricted Project
aeubanks committed rG4e83e59eb8f0: [GVN] Add missing ICF update (authored by aeubanks).
[GVN] Add missing ICF update
Tue, Apr 6, 10:17 AM
aeubanks closed D99909: [GVN] Add missing ICF update.
Tue, Apr 6, 10:16 AM · Restricted Project
aeubanks committed rG9c8b28a69b95: [llvm-reduce] Remove unwanted module inline asm (authored by aeubanks).
[llvm-reduce] Remove unwanted module inline asm
Tue, Apr 6, 9:36 AM
aeubanks closed D99921: [llvm-reduce] Remove unwanted module inline asm.
Tue, Apr 6, 9:35 AM · Restricted Project
aeubanks added inline comments to D99943: [llvm-reduce] Skip dso_local reduction step that results in invalid IR.
Tue, Apr 6, 9:07 AM · Restricted Project

Mon, Apr 5

aeubanks added a comment to D99774: [LoopUtils] Populate sibling loops in reverse program order on new pass manager.

Actually, can you give a concrete example? The example in the description isn't clear enough

Mon, Apr 5, 10:01 PM · Restricted Project
aeubanks added a comment to D99774: [LoopUtils] Populate sibling loops in reverse program order on new pass manager.

It does make sense to me that visiting uses first could be better. I'm not sure if there was actually any specific reason to change the order in the NPM. We'll do some performance testing with this.

Mon, Apr 5, 9:26 PM · Restricted Project
aeubanks updated the summary of D99921: [llvm-reduce] Remove unwanted module inline asm.
Mon, Apr 5, 9:20 PM · Restricted Project
aeubanks updated the diff for D99921: [llvm-reduce] Remove unwanted module inline asm.

add TODO

Mon, Apr 5, 9:20 PM · Restricted Project
aeubanks added reviewers for D99921: [llvm-reduce] Remove unwanted module inline asm: hans, zequanwu, swamulism.
Mon, Apr 5, 9:11 PM · Restricted Project
aeubanks requested review of D99921: [llvm-reduce] Remove unwanted module inline asm.
Mon, Apr 5, 9:10 PM · Restricted Project
aeubanks committed rGea0e2ca1acb2: [SROA] Allow SROA on pointers with invariant group intrinsic uses (authored by aeubanks).
[SROA] Allow SROA on pointers with invariant group intrinsic uses
Mon, Apr 5, 8:00 PM
aeubanks closed D99760: [SROA] Allow SROA on pointers with invariant group intrinsic uses.
Mon, Apr 5, 7:59 PM · Restricted Project