mcrosier (Chad Rosier)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 12 2013, 7:42 AM (209 w, 3 d)

Recent Activity

Thu, Nov 16

mcrosier added inline comments to D40107: [AArch64] Remove obsoleted feature.
Thu, Nov 16, 5:53 AM

Wed, Nov 15

mcrosier accepted D40090: [AArch64] Refactor the loads and stores optimizer.

Please run clang-format, but otherwise LGTM.

Wed, Nov 15, 11:40 AM

Tue, Nov 14

mcrosier added a comment to D39976: [AArch64] Consider the cost model when folding loads and stores.

Please, let me know if I addressed your concerns before I split the code refactoring in another patch.

Tue, Nov 14, 1:45 PM
mcrosier added a reviewer for D39976: [AArch64] Consider the cost model when folding loads and stores: junbuml.
Tue, Nov 14, 6:49 AM
mcrosier added a comment to D39976: [AArch64] Consider the cost model when folding loads and stores.

The load/store opt pass is already pretty expensive in terms of compile-time. Did you see any compile-time regressions in your testing? Also, what performance results have you collected?

Tue, Nov 14, 6:48 AM

Mon, Nov 13

mcrosier accepted D39915: [clang] Remove redundant return [NFC].
Mon, Nov 13, 10:34 AM · Restricted Project

Fri, Nov 10

mcrosier added a comment to D39830: [DAGCombine] Transform (A + -2.0*B*C) -> (A - (B+B)*C).

This solution doesn't seem very general, it won't catch.

double test2(double a, double b, double c, double d) {
  return a + -2.0*b*c*d;
}

The constant can be many layers of multiplies away. Reassociate pushes constants down the tree. Should reassociate be pulling out the negate when it factors the tree?

Fri, Nov 10, 11:10 AM
mcrosier added inline comments to D39830: [DAGCombine] Transform (A + -2.0*B*C) -> (A - (B+B)*C).
Fri, Nov 10, 9:24 AM
mcrosier added a comment to D39830: [DAGCombine] Transform (A + -2.0*B*C) -> (A - (B+B)*C).

what is the purpose of this transform?

Fri, Nov 10, 9:21 AM

Thu, Nov 9

mcrosier added a comment to D39830: [DAGCombine] Transform (A + -2.0*B*C) -> (A - (B+B)*C).

In the bug report I provided a C test case:

double test2(double a, double b, double c) {
  return a + -2.0*b*c;
}
Thu, Nov 9, 1:49 PM

Thu, Nov 2

mcrosier committed rL317222: [TargetParser][AArch64] Reorder enum to preserve 5.0.0 libLLVM ABI..
[TargetParser][AArch64] Reorder enum to preserve 5.0.0 libLLVM ABI.
Thu, Nov 2, 10:53 AM
mcrosier closed D39558: [TargetParser][AArch64] Reorder enum to preserve 5.0.0 libLLVM ABI. by committing rL317222: [TargetParser][AArch64] Reorder enum to preserve 5.0.0 libLLVM ABI..
Thu, Nov 2, 10:53 AM
mcrosier added a comment to D39558: [TargetParser][AArch64] Reorder enum to preserve 5.0.0 libLLVM ABI..

Why did they change in the first place?

The reordering was arbitrary, but I didn't realize the implications at the time. :/

Been there, done that. :)

I don't think we need tests for testing the ordering. The original patch already had tests, we should be fine.

Also, this would be good to be on trunk as well as 5.0. We can't break binary compatibility between dot-releases, but we shouldn't do so on normal releases either, unless there's a reason.

Thu, Nov 2, 9:56 AM
mcrosier added a comment to D39558: [TargetParser][AArch64] Reorder enum to preserve 5.0.0 libLLVM ABI..

Why did they change in the first place?

Thu, Nov 2, 9:45 AM
mcrosier added a comment to D39558: [TargetParser][AArch64] Reorder enum to preserve 5.0.0 libLLVM ABI..

Would it make sense to add a test, so that any future changes doesn't undo this behaviour?

Thu, Nov 2, 9:43 AM
mcrosier created D39558: [TargetParser][AArch64] Reorder enum to preserve 5.0.0 libLLVM ABI..
Thu, Nov 2, 8:54 AM

Mon, Oct 30

mcrosier accepted D39424: [Reassociate] Remove FIXME from looptest.ll (NFC).

This probably comes from when I was adding support for FP in the reassociation pass many years ago. I don't recall exactly why I added the FIXME, but the original test was checking that (i+j) gets reassociated, so I agree with Florian's change.

Mon, Oct 30, 10:26 AM

Thu, Oct 26

mcrosier added a comment to D39137: Add CallSiteSplitting pass.

I'll have a closer look later.

For now, CC @sdesmalen , he worked on a similar pass.

Thu, Oct 26, 7:29 AM
mcrosier added inline comments to D39137: Add CallSiteSplitting pass.
Thu, Oct 26, 7:23 AM

Mon, Oct 23

mcrosier added a comment to D39137: Add CallSiteSplitting pass.

Graham,
This is the inlining work we discussed at the dev meeting. Would you mind adding the Manchester developer that work on the related patch internally?

Mon, Oct 23, 7:23 AM

Oct 13 2017

mcrosier closed D38863: Typos in tutorial.

Committed in r315652.

Oct 13 2017, 6:51 AM

Oct 6 2017

mcrosier retitled D38641: [Inline][WIP] Expose more inlining opportunities by further constraining call site arguments based on splitting an OR condition. from [Inline][WIP] Expose more inlining opportunities by further constraining call site arguments based on an splitting an OR condition. to [Inline][WIP] Expose more inlining opportunities by further constraining call site arguments based on splitting an OR condition..
Oct 6 2017, 2:50 PM
mcrosier retitled D38641: [Inline][WIP] Expose more inlining opportunities by further constraining call site arguments based on splitting an OR condition. from [Inline][WIP] Try to inline if predicated on OR condition to [Inline][WIP] Expose more inlining opportunities by further constraining call site arguments based on an splitting an OR condition..
Oct 6 2017, 2:48 PM

Oct 2 2017

mcrosier added a comment to D38448: [AsmParser] Support GAS's .print directive.

LGTM.

Oct 2 2017, 6:29 AM
mcrosier accepted D38448: [AsmParser] Support GAS's .print directive.

LGTM.

Oct 2 2017, 6:28 AM

Sep 29 2017

mcrosier added inline comments to D33946: [InlineCost] Find identical loads in the callee.
Sep 29 2017, 1:01 PM

Sep 27 2017

mcrosier added a comment to D38263: [InstCombine] Gating select arithmetic optimization.

Do you have commit access, btw? I can commit if not.

Sep 27 2017, 10:32 AM
mcrosier committed rL314320: [InstCombine] Gating select arithmetic optimization..
[InstCombine] Gating select arithmetic optimization.
Sep 27 2017, 10:18 AM
mcrosier closed D38263: [InstCombine] Gating select arithmetic optimization by committing rL314320: [InstCombine] Gating select arithmetic optimization..
Sep 27 2017, 10:18 AM
mcrosier accepted D38263: [InstCombine] Gating select arithmetic optimization.

Do you have commit access, btw? I can commit if not.

Sep 27 2017, 9:38 AM
mcrosier added a comment to D38263: [InstCombine] Gating select arithmetic optimization.

Chad, would like me to amend the test file to include the changes you and
Balaram made?

Sep 27 2017, 9:37 AM
mcrosier retitled D38263: [InstCombine] Gating select arithmetic optimization from Gating select arithmetic optimization to [InstCombine] Gating select arithmetic optimization.
Sep 27 2017, 7:26 AM
mcrosier added a reviewer for D38263: [InstCombine] Gating select arithmetic optimization: mcrosier.
Sep 27 2017, 7:25 AM
mcrosier accepted D38301: [AArch64][Falkor] Ignore SP based loads in HW prefetch fixups..

LGTM.

Sep 27 2017, 6:28 AM
mcrosier added a comment to D38263: [InstCombine] Gating select arithmetic optimization.

LGTM, but I'll defer to David and Quentin for final approval.

Sep 27 2017, 6:13 AM
mcrosier added inline comments to D38288: [InstCombine] Restrict transforming shared selects using SimplifySelectsFeedingBinaryOp when we cannot simplify the binary op..
Sep 27 2017, 6:12 AM

Sep 26 2017

mcrosier added a comment to D38288: [InstCombine] Restrict transforming shared selects using SimplifySelectsFeedingBinaryOp when we cannot simplify the binary op..

LGTM, but I'll defer to @majnemer and company for the final approval.

Sep 26 2017, 3:30 PM
mcrosier added inline comments to D38288: [InstCombine] Restrict transforming shared selects using SimplifySelectsFeedingBinaryOp when we cannot simplify the binary op..
Sep 26 2017, 3:30 PM
mcrosier accepted D38256: [AArch64][Falkor] Fix correctness bug in falkor prefetcher fix pass and correct some opcode tag computations..

LGTM.

Sep 26 2017, 3:30 PM
mcrosier added a comment to D37019: Add select simplifications.

Unfortunately, we're seeing a 4.7% regression in SPEC2006/lbm due to this change. The number of dynamic instructions is increased by ~5.5%.

We're running on AArch64/Falkor with the following flags: -O3 -fno-math-errno -ffp-contract=fast -fomit-frame-pointer

All of the code changes are in LBM_performStreamCollide(), which accounts for ~98% of execution.

I'll keep digging so I can provide more details, but I wanted to report the regression.

Sep 26 2017, 3:30 PM
mcrosier added a comment to D38288: [InstCombine] Restrict transforming shared selects using SimplifySelectsFeedingBinaryOp when we cannot simplify the binary op..

I think we're on the right track, but what about this test?

Sep 26 2017, 3:30 PM
mcrosier added a comment to D37019: Add select simplifications.

Unfortunately, we're seeing a 4.7% regression in SPEC2006/lbm due to this change. The number of dynamic instructions is increased by ~5.5%.

Sep 26 2017, 3:30 PM
mcrosier added a comment to D38263: [InstCombine] Gating select arithmetic optimization.

Please use the -U option to include full context in your diffs (e.g., git show HEAD -U999999 > mypatch.patch).

Sep 26 2017, 3:30 PM
mcrosier updated subscribers of D38256: [AArch64][Falkor] Fix correctness bug in falkor prefetcher fix pass and correct some opcode tag computations..
Sep 26 2017, 3:30 PM
mcrosier accepted D38261: [AArch64][Falkor] Fix bug in falkor prefetcher fix pass..

LGTM.

Sep 26 2017, 3:30 PM

Sep 25 2017

mcrosier added a comment to D38204: [TargetTransformInfo] Check if function pointer is valid before calling isLoweredToCall.

Also, please be sure to include full context with your patches. This is generally done with -U git option (e.g., git show HEAD -U999999 > mypatch.patch).

Sep 25 2017, 8:03 AM
mcrosier added a comment to D38204: [TargetTransformInfo] Check if function pointer is valid before calling isLoweredToCall.

Would it make sense to also add an assertion in isLoweredToCall asserting that F is non-null?

Sep 25 2017, 7:59 AM
mcrosier committed rL314105: [AArch64] Add basic support for Qualcomm's Saphira CPU..
[AArch64] Add basic support for Qualcomm's Saphira CPU.
Sep 25 2017, 7:06 AM
mcrosier abandoned D38192: [TargetParser][AArch64] Add support for SPE feature in the target parser..
Sep 25 2017, 7:04 AM
mcrosier created D38192: [TargetParser][AArch64] Add support for SPE feature in the target parser..
Sep 25 2017, 5:02 AM
mcrosier updated the diff for D38192: [TargetParser][AArch64] Add support for SPE feature in the target parser..

-Add full context.

Sep 25 2017, 5:02 AM

Sep 20 2017

mcrosier edited reviewers for D37821: [SimplifyCfg] Don't sink loads/stores to geps of allocas, added: chandlerc; removed: jmolloy.

+Chandler (SROA owner)
-James (not active AFAICT)

Sep 20 2017, 7:07 AM

Sep 19 2017

mcrosier accepted D37988: [AArch64] Improve tests of loads and stores of register pairs.

Thanks, Evandro.

Sep 19 2017, 6:15 AM

Sep 18 2017

mcrosier resigned from D34566: [loop idiom Recognition] support memcpy for multiple consecutive loads and stores.
Sep 18 2017, 8:16 AM

Sep 15 2017

mcrosier accepted D37922: [llvm] Fix some typos. NFC..

LGTM. Thanks, Mandeep. These type of changes generally don't require pre-approval, btw.

Sep 15 2017, 12:57 PM
mcrosier removed a reviewer for D37896: [DAGCombine] Resolving PR34474 by transforming mul(x, 2^c +/- 1) -> sub/add(shl(x, c) x) for any type including vector types: llvm-commits.
Sep 15 2017, 12:37 PM
mcrosier added a comment to D37896: [DAGCombine] Resolving PR34474 by transforming mul(x, 2^c +/- 1) -> sub/add(shl(x, c) x) for any type including vector types.

Both ARM and AArch64 have similar combines in ISelLowering.cpp. It would be nice if these were moved to the target independent code, if possible.

Sep 15 2017, 12:17 PM

Sep 14 2017

mcrosier added a comment to D33946: [InlineCost] Find identical loads in the callee.

(I suspect most folks are waiting for chandler to say whether he feels this is good enough at this point or not)

Sep 14 2017, 8:56 AM
mcrosier committed rL313263: Add newline to end of test file. NFC..
Add newline to end of test file. NFC.
Sep 14 2017, 7:50 AM

Sep 13 2017

mcrosier added inline comments to D37198: [InlineCost] add visitSelectInst().
Sep 13 2017, 9:53 AM

Sep 1 2017

mcrosier added inline comments to D33946: [InlineCost] Find identical loads in the callee.
Sep 1 2017, 11:07 AM

Aug 28 2017

mcrosier added a comment to D37198: [InlineCost] add visitSelectInst().

Overall, the logic of the patch is in good shape. However, I'd suggest some minor refactoring to delineate the select of constants/values vs. GEPs for SROA.

Aug 28 2017, 8:54 AM

Aug 24 2017

mcrosier committed rL311702: [PartialInlining] Formatting. NFC..
[PartialInlining] Formatting. NFC.
Aug 24 2017, 2:24 PM
mcrosier committed rL311699: [PartialInlining] Type. NFC..
[PartialInlining] Type. NFC.
Aug 24 2017, 1:32 PM
mcrosier added a comment to D37106: [Driver][AArch64] Tests for rdm feature..

Thanks for fixing this.

Aug 24 2017, 8:19 AM
mcrosier accepted D34879: [LoopInterchange] Skip zext instructions when looking for induction var..

Seems reasonable to me assuming the usual bit of testing has been done.

Aug 24 2017, 7:42 AM
mcrosier committed rL311660: [Driver][AArch64] Add tests for RDM feature..
[Driver][AArch64] Add tests for RDM feature.
Aug 24 2017, 7:34 AM
mcrosier closed D37106: [Driver][AArch64] Tests for rdm feature. by committing rL311660: [Driver][AArch64] Add tests for RDM feature..
Aug 24 2017, 7:34 AM
mcrosier committed rL311659: [TargetParser][AArch64] Add support for RDM feature in the target parser..
[TargetParser][AArch64] Add support for RDM feature in the target parser.
Aug 24 2017, 7:32 AM
mcrosier closed D37081: [TargetParser][AArch64] Add support for RDM feature in the target parser. by committing rL311659: [TargetParser][AArch64] Add support for RDM feature in the target parser..
Aug 24 2017, 7:32 AM
mcrosier added inline comments to D37081: [TargetParser][AArch64] Add support for RDM feature in the target parser..
Aug 24 2017, 6:26 AM
mcrosier added a comment to D37081: [TargetParser][AArch64] Add support for RDM feature in the target parser..

Looks good to me, was wondering if you're also planning to add a Clang patch to test +rdm?

Yes, we should definitely add those. Coming up shortly.

Aug 24 2017, 6:23 AM
mcrosier created D37106: [Driver][AArch64] Tests for rdm feature..
Aug 24 2017, 6:23 AM
mcrosier updated the diff for D37081: [TargetParser][AArch64] Add support for RDM feature in the target parser..

-Add +rdm to llvm::AArch64::getExtensionFeatures. This was exposed while trying to write clang/driver tests. Thanks, Sjoerd.

Aug 24 2017, 6:20 AM
mcrosier added a comment to D37081: [TargetParser][AArch64] Add support for RDM feature in the target parser..

Looks good to me, was wondering if you're also planning to add a Clang patch to test +rdm?

Aug 24 2017, 5:54 AM
mcrosier updated the diff for D37081: [TargetParser][AArch64] Add support for RDM feature in the target parser..

Address Sam's feedback by adding a few more tests.

Aug 24 2017, 5:54 AM

Aug 23 2017

mcrosier created D37081: [TargetParser][AArch64] Add support for RDM feature in the target parser..
Aug 23 2017, 2:30 PM
mcrosier edited reviewers for D37051: Model cache size and associativity in TargetTransformInfo, added: efriedma; removed: eli.friedman.
Aug 23 2017, 7:31 AM
mcrosier committed rL311554: [Reassociate] Don't canonicalize x + (-Constant * y) -> x - (Constant * y)...
[Reassociate] Don't canonicalize x + (-Constant * y) -> x - (Constant * y)..
Aug 23 2017, 7:13 AM

Aug 22 2017

mcrosier resigned from D36793: [X86AsmParser] Refactoring, (almost) NFC..
Aug 22 2017, 7:28 AM

Aug 21 2017

mcrosier committed rL311370: [InlineCost] Add more debug during inline cost computation..
[InlineCost] Add more debug during inline cost computation.
Aug 21 2017, 12:59 PM
mcrosier accepted D35850: [InlineCost] Add cl::opt to allow full inline cost to be computed for debugging purposes..

LGTM.

Aug 21 2017, 9:23 AM

Aug 18 2017

mcrosier accepted D36807: [AArch64] Restore the test of conditional branch fusion.

Thanks, Evandro.

Aug 18 2017, 12:36 PM
mcrosier abandoned D36591: [PredicateInfo] Add a helper routine to remove SSA copies from a Function..

We can revisit this once there's a use case.

Aug 18 2017, 9:55 AM
mcrosier added inline comments to D36844: [PGO] Fixed assertion due to mismatched memcpy size type..
Aug 18 2017, 9:31 AM
mcrosier added inline comments to D35850: [InlineCost] Add cl::opt to allow full inline cost to be computed for debugging purposes..
Aug 18 2017, 9:21 AM

Aug 14 2017

mcrosier updated the summary of D36656: [SCCP] Propagate integer range info for parameters in IPSCCP..
Aug 14 2017, 9:57 AM

Aug 11 2017

mcrosier added a comment to D36591: [PredicateInfo] Add a helper routine to remove SSA copies from a Function..

I'm not necessarily against this, and I think having PI garbage collecting the intrinsics is a good thing.
What I'm wondering here if you hit a case when this actually matters? I and Dan debugged several cases where PI intrinsics weren't reclaimed but those were all legitimate bugs in NewGVN.
In case we decide to use PI for some other pass (LVI is an obvious candidate) then having this logic handled in the utility may make things easier.

Aug 11 2017, 5:55 AM

Aug 10 2017

mcrosier created D36591: [PredicateInfo] Add a helper routine to remove SSA copies from a Function..
Aug 10 2017, 2:11 PM
mcrosier closed D36539: [NewGVN] Add command-line option to control the generation of phi-of-ops (disable by default).

r310594

Aug 10 2017, 7:21 AM
mcrosier committed rL310594: [NewGVN] Add CL option to control the generation of phi-of-ops (disable by….
[NewGVN] Add CL option to control the generation of phi-of-ops (disable by…
Aug 10 2017, 7:16 AM

Aug 9 2017

mcrosier retitled D36539: [NewGVN] Add command-line option to control the generation of phi-of-ops (disable by default) from [NewGVN] Add command-line option to control the generation of phi-of-ops. NFC. to [NewGVN] Add command-line option to control the generation of phi-of-ops (disable by default).
Aug 9 2017, 2:03 PM
mcrosier updated the diff for D36539: [NewGVN] Add command-line option to control the generation of phi-of-ops (disable by default).

-Disable this feature by default.
-Update test RUN commands due to change.

Aug 9 2017, 2:03 PM
mcrosier created D36539: [NewGVN] Add command-line option to control the generation of phi-of-ops (disable by default).
Aug 9 2017, 12:17 PM
mcrosier added inline comments to D36432: [IPSCCP] Add function specialization ability.
Aug 9 2017, 11:35 AM
mcrosier added reviewers for D36517: [AArch64] Enable ARMv8.3-A pointer authentication: t.p.northover, rengolin, SjoerdMeijer, john.brawn.
Aug 9 2017, 8:56 AM
mcrosier added inline comments to D36512: [ValueTracking] Honour recursion limit.
Aug 9 2017, 8:12 AM
mcrosier updated subscribers of D36504: [CodeGenPrepare][WIP] Convert uncond. branch to return into a return to help with shrink-wrapping.
Aug 9 2017, 8:09 AM
mcrosier accepted D36512: [ValueTracking] Honour recursion limit.

One small comment, but otherwise LGTM. Thanks, Davide.

Aug 9 2017, 7:57 AM
mcrosier added a comment to D35850: [InlineCost] Add cl::opt to allow full inline cost to be computed for debugging purposes..

@haicheng has offered to take up this work!

Aug 9 2017, 6:38 AM