Page MenuHomePhabricator

john.brawn (John Brawn)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 12 2015, 7:44 AM (244 w, 4 d)

Recent Activity

Thu, Oct 3

john.brawn committed rGb44204c77e63: [libunwind] Adjust libunwind_01.pass.cpp test for ARM EHABI (authored by john.brawn).
[libunwind] Adjust libunwind_01.pass.cpp test for ARM EHABI
Thu, Oct 3, 10:01 AM
john.brawn committed rL373628: [libunwind] Adjust libunwind_01.pass.cpp test for ARM EHABI.
[libunwind] Adjust libunwind_01.pass.cpp test for ARM EHABI
Thu, Oct 3, 10:01 AM
john.brawn closed D68387: [libunwind] Adjust libunwind_01.pass.cpp test for ARM EHABI.
Thu, Oct 3, 10:01 AM · Restricted Project
john.brawn created D68387: [libunwind] Adjust libunwind_01.pass.cpp test for ARM EHABI.
Thu, Oct 3, 6:20 AM · Restricted Project

Mon, Sep 30

john.brawn committed rL373203: Request commit access got john.brawn.
Request commit access got john.brawn
Mon, Sep 30, 5:49 AM

Jul 17 2019

john.brawn accepted D64757: [PEI] Don't re-allocate a pre-allocated stack protector slot.

Tests look good to me.

Jul 17 2019, 3:20 AM · Restricted Project

Jul 16 2019

john.brawn added inline comments to D64759: [CodeGen] Don't resolve the stack protector frame accesses until PEI.
Jul 16 2019, 4:45 AM · Restricted Project
john.brawn added a comment to D64757: [PEI] Don't re-allocate a pre-allocated stack protector slot.

This will fix https://bugs.llvm.org/show_bug.cgi?id=25610, so it would be good if we could have a test for that as well (and to close that bug once this is committed).

Jul 16 2019, 3:44 AM · Restricted Project

Jul 5 2019

john.brawn accepted D64242: [AArch64] Fix scalar vuqadd intrinsics operands.

LGTM, with one nitpick.

Jul 5 2019, 7:02 AM · Restricted Project, Restricted Project
john.brawn accepted D64239: [AArch64] Fix vsqadd scalar intrinsics operands.

LGTM, with a couple of nitpicks.

Jul 5 2019, 6:31 AM · Restricted Project, Restricted Project

Jun 12 2019

john.brawn accepted D63148: [MIR] Skip hoisting to throwable or return machine basic blocks.

LGTM.

Jun 12 2019, 4:48 AM · Restricted Project

Jun 11 2019

john.brawn reopened D56772: [MIR] Add simple PRE pass to MachineCSE.

This is causing incorrect code generation for this piece of IR:

Jun 11 2019, 4:35 AM · Restricted Project

May 22 2019

john.brawn committed rG6c49f58a355b: [ARM][AArch64] Fix incorrect handling of alignment in va_arg code generation (authored by john.brawn).
[ARM][AArch64] Fix incorrect handling of alignment in va_arg code generation
May 22 2019, 4:41 AM
john.brawn committed rL361372: [ARM][AArch64] Fix incorrect handling of alignment in va_arg code generation.
[ARM][AArch64] Fix incorrect handling of alignment in va_arg code generation
May 22 2019, 4:41 AM
john.brawn committed rC361372: [ARM][AArch64] Fix incorrect handling of alignment in va_arg code generation.
[ARM][AArch64] Fix incorrect handling of alignment in va_arg code generation
May 22 2019, 4:41 AM
john.brawn closed D62152: [ARM][AArch64] Fix incorrect handling of alignment in va_arg code generation.
May 22 2019, 4:41 AM · Restricted Project

May 21 2019

john.brawn added a comment to D62152: [ARM][AArch64] Fix incorrect handling of alignment in va_arg code generation.

Please verify my understanding of the following is correct:

  1. getTypeUnadjustedAlign() is currently only used to compute calling convention alignment for ARM and AArch64.

Yes, the only places it's called are AArch64ABIInfo::classifyArgumentType and ARMABIInfo::classifyArgumentType.

May 21 2019, 7:22 AM · Restricted Project

May 20 2019

john.brawn created D62152: [ARM][AArch64] Fix incorrect handling of alignment in va_arg code generation.
May 20 2019, 9:42 AM · Restricted Project

Mar 8 2019

john.brawn added a comment to D59080: Merge of global constants does not happen when constants have common linkage.

The title and summary of this patch describe a problem, but they don't describe how this patch fixes that problem (which makes it hard to figure out if this patch is correctly fixing the problem). It should be more like "[GlobalMerge] Do XYZ: We have problem ABC, doing XYZ in GlobalMerge fixes it because ...". Also this patch needs tests.

Mar 8 2019, 3:37 AM

Feb 26 2019

john.brawn accepted D58348: [AArch64] Fix for bug 35094 atomicrmw on Armv8.1-A+lse.

LGTM.

Feb 26 2019, 8:00 AM · Restricted Project

Feb 25 2019

john.brawn accepted D58490: [ARM] Be super conservative about atomics.

LGTM.

Feb 25 2019, 10:28 AM · Restricted Project
john.brawn added inline comments to D58490: [ARM] Be super conservative about atomics.
Feb 25 2019, 4:40 AM · Restricted Project

Feb 21 2019

john.brawn added inline comments to D58348: [AArch64] Fix for bug 35094 atomicrmw on Armv8.1-A+lse.
Feb 21 2019, 5:08 AM · Restricted Project

Feb 18 2019

john.brawn requested changes to D57820: [AArch64] Use CAS loops for all atomic operations when available..

It's a bit hard to know whether to accept this or not, given that you says it's "probably better to avoid LDXR". Do you have any data to say if it is any better or not?

Feb 18 2019, 4:33 AM · Restricted Project

Feb 15 2019

john.brawn added a comment to D57820: [AArch64] Use CAS loops for all atomic operations when available..

It's a bit hard to know whether to accept this or not, given that you says it's "probably better to avoid LDXR". Do you have any data to say if it is any better or not?

Feb 15 2019, 10:08 AM · Restricted Project
john.brawn accepted D58166: [BPI] Look through bitcasts in calcZeroHeuristic.

LGTM. Looking at it it does make me wonder if we should handle things that aren't ConstantInt, e.g.

bitcast <2 x i16> <i16 0, i16 0> to i32

is still zero, though I'm guessing things like this actually probably never happen and this commit is fine as it is anyway.

Feb 15 2019, 3:39 AM · Restricted Project

Jan 4 2019

john.brawn committed rL350408: [LICM] Adjust how moving the re-hoist point works.
[LICM] Adjust how moving the re-hoist point works
Jan 4 2019, 9:16 AM
john.brawn closed D55266: [LICM] Use per-block hoist points when rehoisting instructions.
Jan 4 2019, 9:15 AM

Dec 14 2018

john.brawn committed rL349151: [RegAllocGreedy] IMPLICIT_DEF values shouldn't prefer registers.
[RegAllocGreedy] IMPLICIT_DEF values shouldn't prefer registers
Dec 14 2018, 6:11 AM
john.brawn closed D55652: [RegAllocGreedy] IMPLICIT_DEF values shouldn't prefer registers.
Dec 14 2018, 6:11 AM

Dec 13 2018

john.brawn created D55652: [RegAllocGreedy] IMPLICIT_DEF values shouldn't prefer registers.
Dec 13 2018, 6:11 AM

Dec 10 2018

john.brawn created D55512: [MemDep] Adjust NonLocalPointerDep handling to be more precise..
Dec 10 2018, 7:39 AM

Dec 7 2018

john.brawn added inline comments to D53706: [MultiTailCallElimination]: Pass to eliminate multiple tail calls.
Dec 7 2018, 7:54 AM
john.brawn added inline comments to D53706: [MultiTailCallElimination]: Pass to eliminate multiple tail calls.
Dec 7 2018, 5:23 AM

Dec 6 2018

john.brawn updated the diff for D55266: [LICM] Use per-block hoist points when rehoisting instructions.

Adjust according to review comments, and also add some more tests.

Dec 6 2018, 7:56 AM

Dec 5 2018

john.brawn accepted D55009: [GVN] Don't perform scalar PRE on GEPs.

OK, then this looks good to me.

Dec 5 2018, 10:08 AM

Dec 4 2018

john.brawn added inline comments to D55009: [GVN] Don't perform scalar PRE on GEPs.
Dec 4 2018, 8:17 AM
john.brawn added a comment to D55009: [GVN] Don't perform scalar PRE on GEPs.

The idea behind allowing Scalar PRE when the addressing mode isn't legal is for some thing like (adjusted from pre-gep-load.ll)

define void @foo(i32 %stat, i32 %i, double* %p, double* %q) {
entry:
  switch i32 %stat, label %sw.default [
    i32 0, label %sw.bb
    i32 1, label %sw.bb
    i32 2, label %sw.bb2
  ]
Dec 4 2018, 8:09 AM
john.brawn created D55266: [LICM] Use per-block hoist points when rehoisting instructions.
Dec 4 2018, 6:14 AM

Nov 29 2018

john.brawn committed rL347889: [LICM] Reapply r347776 "Make LICM able to hoist phis" with fix.
[LICM] Reapply r347776 "Make LICM able to hoist phis" with fix
Nov 29 2018, 9:13 AM

Nov 28 2018

john.brawn added a comment to D55009: [GVN] Don't perform scalar PRE on GEPs.

It may be worthwhile allowing scalar PRE on GEPs that we know won't be combined into the addressing mode of a load/store, i.e. those where TargetTransformInfo::isLegalAddressingMode returns false.

Nov 28 2018, 10:01 AM
john.brawn committed rL347778: [LICM] Enable control flow hoisting by default.
[LICM] Enable control flow hoisting by default
Nov 28 2018, 9:26 AM
john.brawn closed D54949: [LICM] Enable control flow hoisting by default.
Nov 28 2018, 9:26 AM
john.brawn committed rL347776: [LICM] Reapply r347190 "Make LICM able to hoist phis" with fix.
[LICM] Reapply r347190 "Make LICM able to hoist phis" with fix
Nov 28 2018, 9:24 AM
john.brawn closed D52827: [LICM] Make LICM able to hoist phis.
Nov 28 2018, 9:24 AM
john.brawn accepted D54932: [CGP] Improve compile time for complex addressing mode.

LGTM.

Nov 28 2018, 8:50 AM

Nov 27 2018

john.brawn added a comment to D54932: [CGP] Improve compile time for complex addressing mode.

This looks good, but I think that some of the comments need to be adjusted as they're no longer accurate for what the code is doing.

Nov 27 2018, 7:15 AM
john.brawn added a child revision for D52827: [LICM] Make LICM able to hoist phis: D54949: [LICM] Enable control flow hoisting by default.
Nov 27 2018, 6:31 AM
john.brawn added a parent revision for D54949: [LICM] Enable control flow hoisting by default: D52827: [LICM] Make LICM able to hoist phis.
Nov 27 2018, 6:31 AM
john.brawn created D54949: [LICM] Enable control flow hoisting by default.
Nov 27 2018, 6:30 AM
john.brawn updated the diff for D52827: [LICM] Make LICM able to hoist phis.

Added an option to enable control flow hoisting, turned off by default.

Nov 27 2018, 6:29 AM

Nov 26 2018

john.brawn accepted D54517: [CGP] Limit Complex Addressing mode by number of BasicBlocks to traverse.

Looks OK as a fix for the reported bug, with one minor nitpick. Taking a look at PR39625 it looks like there's an underlying problem where we insert a load of useless placeholders, e.g. for

declare void @otherfn()
Nov 26 2018, 7:40 AM

Nov 22 2018

john.brawn added a comment to D52827: [LICM] Make LICM able to hoist phis.

This rehoisting stuff has been wary for me before, and now I'm completely hesitant about what is going on there. Why do we end up hoisting to some block that does not dominate its users? Do we have a strong reason of doing so rather than finding the correct hoisting destination?

Nov 22 2018, 6:20 AM
john.brawn committed rL347456: [AArch64] Fix SelectionDAG infinite loop for v1i64 SCALAR_TO_VECTOR.
[AArch64] Fix SelectionDAG infinite loop for v1i64 SCALAR_TO_VECTOR
Nov 22 2018, 3:48 AM

Nov 20 2018

john.brawn requested review of D52827: [LICM] Make LICM able to hoist phis.
Nov 20 2018, 9:13 AM
john.brawn reopened D52827: [LICM] Make LICM able to hoist phis.
Nov 20 2018, 9:12 AM
john.brawn updated the diff for D52827: [LICM] Make LICM able to hoist phis.

The thing causing the crash was incorrect handling of instructions that need to be rehoisted. When we have hoisted a phi, then hoisted a use of that phi, then if have to rehoist that use we need to make sure that we rehoist it to _after_ the hoisted phi. We can do this by rehoisting to the immediate dominator instead of just rehoisting everything to the original preheader.

Nov 20 2018, 9:12 AM

Nov 19 2018

john.brawn committed rL347190: [LICM] Make LICM able to hoist phis.
[LICM] Make LICM able to hoist phis
Nov 19 2018, 3:34 AM
john.brawn closed D52827: [LICM] Make LICM able to hoist phis.
Nov 19 2018, 3:34 AM

Nov 14 2018

john.brawn committed rL346869: [SimplifyCFG] Regenerate preserve-branchweights.ll test. NFC.
[SimplifyCFG] Regenerate preserve-branchweights.ll test. NFC
Nov 14 2018, 7:29 AM
john.brawn updated the diff for D52827: [LICM] Make LICM able to hoist phis.

Update according to review comments.

Nov 14 2018, 6:01 AM
john.brawn added a comment to D52827: [LICM] Make LICM able to hoist phis.

The patch fails the following test:

...

Likely some corner case related to conditional branch to the same block for true and false is not handled. Please make sure that it passes before we can proceed with the patch.

Nov 14 2018, 5:47 AM

Nov 13 2018

john.brawn added a comment to D53706: [MultiTailCallElimination]: Pass to eliminate multiple tail calls.

Also, we should be disabling this transformation when optimising for size.

Nov 13 2018, 6:05 AM
john.brawn added a comment to D53706: [MultiTailCallElimination]: Pass to eliminate multiple tail calls.

A few general comments:

Nov 13 2018, 6:00 AM

Nov 8 2018

john.brawn updated the diff for D52827: [LICM] Make LICM able to hoist phis.

Use SmallVector for HoistedInstructions, rename getHoistedBlock, add LI and DT verification, other small changes.

Nov 8 2018, 5:22 AM

Nov 7 2018

john.brawn added inline comments to D52827: [LICM] Make LICM able to hoist phis.
Nov 7 2018, 8:46 AM

Nov 6 2018

john.brawn updated the diff for D52827: [LICM] Make LICM able to hoist phis.

Use LoopBlocksRPO to generate the worklist.

Nov 6 2018, 10:12 AM
john.brawn added a comment to D52827: [LICM] Make LICM able to hoist phis.

I generally like what this patch is doing, but the size of code and its complexity overwhelms me. Is it possible to separate the patch into smaller pieces so that it would be easier to review them in isolation? If yes, I'd appreciate that a lot.

Nov 6 2018, 10:07 AM
john.brawn added inline comments to D52827: [LICM] Make LICM able to hoist phis.
Nov 6 2018, 8:39 AM

Nov 5 2018

john.brawn added a comment to D52550: [ARM] Check for sel intrinsic use in ARM CGP.

Because the ABI says that the GE bits are undefined on entry / return from public interfaces (i.e. non-static functions) I think that it is possible to just look at the current translation unit and make a decision based on that. I'm not sure if just looking at whether the sel intrinsic is used will work though - the GE bits could be read by __arm_rsr("APSR"), or by inline assembly, and maybe there are other situations that I haven't thought of.

Nov 5 2018, 8:20 AM
john.brawn accepted D52653: [CodeGen, AArch64] Combine Interleaved Loads which are not covered by the Vectorizer.

A few errors in comments, but otherwise LGTM so long as you fix those errors before committing.

Nov 5 2018, 5:04 AM

Oct 31 2018

john.brawn added a comment to D52827: [LICM] Make LICM able to hoist phis.

Does this lose debug information? I do very little with optimization passes so I don't know, but it would be nice to keep it in mind.

Oct 31 2018, 9:17 AM
john.brawn updated the diff for D52827: [LICM] Make LICM able to hoist phis.

Update according to review comments.

Oct 31 2018, 9:17 AM
john.brawn added inline comments to D52827: [LICM] Make LICM able to hoist phis.
Oct 31 2018, 8:55 AM

Oct 25 2018

john.brawn committed rL345275: [AArch64] Add EXT patterns for 64-bit EXT of a subvector of a 128-bit vector.
[AArch64] Add EXT patterns for 64-bit EXT of a subvector of a 128-bit vector
Oct 25 2018, 8:34 AM
john.brawn closed D53582: [AArch64] Add EXT patterns for 64-bit EXT of a subvector of a 128-bit vector.
Oct 25 2018, 8:34 AM
john.brawn committed rL345271: [AArch64] Refactor definition of EXT patterns to use a multiclass.
[AArch64] Refactor definition of EXT patterns to use a multiclass
Oct 25 2018, 8:03 AM
john.brawn closed D53580: [AArch64] Refactor definition of EXT patterns to use a multiclass.
Oct 25 2018, 8:03 AM
john.brawn committed rL345270: [AArch64] Do 64-bit vector move of 0 and -1 by extracting from the 128-bit move.
[AArch64] Do 64-bit vector move of 0 and -1 by extracting from the 128-bit move
Oct 25 2018, 7:59 AM
john.brawn closed D53579: [AArch64] Do 64-bit vector move of 0 and -1 by extracting from the 128-bit move.
Oct 25 2018, 7:59 AM

Oct 24 2018

john.brawn added a comment to D52227: LLVM: Expose SimplifyCFGPass (as used in PassBuilder::buildModuleOptimizationPipeline) in PassRegistry.def.

Having some way to generate a pipeline string which matches what opt does by default is definitely a good thing. I think that long-term splitting up simplifycfg is probably the best way, and I think I'm going to have to do some of that myself as part of D50723 (as the current implementation causes a lot of problems for bugpoint as the pass list it deduces doesn't do the same thing as -Owhatever), but doing it in this way for now seems fine.

Oct 24 2018, 10:09 AM

Oct 23 2018

john.brawn added parent revisions for D53582: [AArch64] Add EXT patterns for 64-bit EXT of a subvector of a 128-bit vector: D53579: [AArch64] Do 64-bit vector move of 0 and -1 by extracting from the 128-bit move, D53580: [AArch64] Refactor definition of EXT patterns to use a multiclass.
Oct 23 2018, 8:30 AM
john.brawn added a child revision for D53580: [AArch64] Refactor definition of EXT patterns to use a multiclass: D53582: [AArch64] Add EXT patterns for 64-bit EXT of a subvector of a 128-bit vector.
Oct 23 2018, 8:30 AM
john.brawn added a child revision for D53579: [AArch64] Do 64-bit vector move of 0 and -1 by extracting from the 128-bit move: D53582: [AArch64] Add EXT patterns for 64-bit EXT of a subvector of a 128-bit vector.
Oct 23 2018, 8:30 AM
john.brawn created D53582: [AArch64] Add EXT patterns for 64-bit EXT of a subvector of a 128-bit vector.
Oct 23 2018, 8:29 AM
john.brawn created D53580: [AArch64] Refactor definition of EXT patterns to use a multiclass.
Oct 23 2018, 8:28 AM
john.brawn created D53579: [AArch64] Do 64-bit vector move of 0 and -1 by extracting from the 128-bit move.
Oct 23 2018, 8:23 AM
john.brawn added a comment to D52827: [LICM] Make LICM able to hoist phis.

Ping.

Oct 23 2018, 8:02 AM
john.brawn added inline comments to D52653: [CodeGen, AArch64] Combine Interleaved Loads which are not covered by the Vectorizer.
Oct 23 2018, 7:00 AM

Oct 12 2018

john.brawn added inline comments to D52653: [CodeGen, AArch64] Combine Interleaved Loads which are not covered by the Vectorizer.
Oct 12 2018, 7:04 AM

Oct 11 2018

john.brawn added a comment to D52827: [LICM] Make LICM able to hoist phis.

Starting with only high level design comments....

I think this patch is going about things in slightly the wrong way. As far as I can tell, there are two key components to this: 1) hoisting PHIs w/loop invariant selectors and operands, and 2) hoisting instructions from below conditional branches into conditional blocks inserted in preheader. I think these two can and should be separate into distinct patches.

I'm concerned about (2). This really seems like a form of loop peeling, and a currently unrestricted one without a cost model. I'm not sure I'm convinced this belongs in LICM at all.

(1) could be formulated w/o (2). The easiest way would be to form a select chain using the LIV conditions and LIV operands. This also works for any instruction for which we have a predicated form.

I'd not considered going direct from phi to select when hoisting. It seems like it will restrict the kinds of phis that can be hoisted, but we're probably not hoisting them anyway due to the various TODOs here. I'll look into it.

Oct 11 2018, 8:20 AM
john.brawn updated the diff for D52827: [LICM] Make LICM able to hoist phis.

Update based on review comments, and add some more tests.

Oct 11 2018, 8:09 AM

Oct 10 2018

john.brawn committed rL344139: [llvm-exegesis] Fix function return generation so it doesn't return register 0.
[llvm-exegesis] Fix function return generation so it doesn't return register 0
Oct 10 2018, 6:05 AM
john.brawn closed D53074: [llvm-exegesis] Fix function return generation so it doesn't return register 0.
Oct 10 2018, 6:05 AM
john.brawn created D53074: [llvm-exegesis] Fix function return generation so it doesn't return register 0.
Oct 10 2018, 5:32 AM

Oct 8 2018

john.brawn added a comment to D52827: [LICM] Make LICM able to hoist phis.

Starting with only high level design comments....

I think this patch is going about things in slightly the wrong way. As far as I can tell, there are two key components to this: 1) hoisting PHIs w/loop invariant selectors and operands, and 2) hoisting instructions from below conditional branches into conditional blocks inserted in preheader. I think these two can and should be separate into distinct patches.

I'm concerned about (2). This really seems like a form of loop peeling, and a currently unrestricted one without a cost model. I'm not sure I'm convinced this belongs in LICM at all.

(1) could be formulated w/o (2). The easiest way would be to form a select chain using the LIV conditions and LIV operands. This also works for any instruction for which we have a predicated form.

Oct 8 2018, 10:32 AM

Oct 5 2018

john.brawn added a comment to D52827: [LICM] Make LICM able to hoist phis.

This has no impact by itself that I've been able to see, as LICM typically doesn't see such phis as they will have been converted into selects by the time LICM is run

That's surprising... I would have expected this to show up in some cases. Some branches can't eliminated due to side-effects.

Oct 5 2018, 9:50 AM

Oct 3 2018

john.brawn created D52827: [LICM] Make LICM able to hoist phis.
Oct 3 2018, 5:06 AM

Sep 18 2018

john.brawn committed rL342471: [TargetLowering] Android has sincos functions.
[TargetLowering] Android has sincos functions
Sep 18 2018, 6:23 AM
john.brawn closed D52025: [TargetLowering] Android has sincos functions.
Sep 18 2018, 6:23 AM