Page MenuHomePhabricator

jmolloy (James Molloy)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 12 2012, 3:50 AM (349 w, 4 d)

Recent Activity

Wed, May 22

jmolloy added a comment to D60982: [SimplifyCFG] Use lookup tables when they are more space efficient or a huge speed win..

Thanks for the context Hans.

Wed, May 22, 1:56 AM · Restricted Project

Mon, May 20

jmolloy accepted D61911: [GlobalOpt] recognize dead struct fields and propagate values.

I think I've convinced myself that this is correct. LGTM.

Mon, May 20, 7:18 AM · Restricted Project

Sun, May 19

jmolloy added inline comments to D60982: [SimplifyCFG] Use lookup tables when they are more space efficient or a huge speed win..
Sun, May 19, 7:10 AM · Restricted Project

Fri, May 10

jmolloy added inline comments to D60982: [SimplifyCFG] Use lookup tables when they are more space efficient or a huge speed win..
Fri, May 10, 5:52 AM · Restricted Project

Thu, May 9

jmolloy requested changes to D60982: [SimplifyCFG] Use lookup tables when they are more space efficient or a huge speed win..
Thu, May 9, 6:14 AM · Restricted Project
jmolloy accepted D61728: [CodeGenPrepare] Limit recursion depth for collectBitParts.

Please clang-format. There should be a space between the + operator and its operands.

Thu, May 9, 6:07 AM · Restricted Project

Fri, May 3

jmolloy requested changes to D60982: [SimplifyCFG] Use lookup tables when they are more space efficient or a huge speed win..

Looking much better. I think the TTI hook could be described better.

Fri, May 3, 7:50 AM · Restricted Project

Thu, May 2

jmolloy requested changes to D60982: [SimplifyCFG] Use lookup tables when they are more space efficient or a huge speed win..

Undo approval; was looking at an incorrect diff.

Thu, May 2, 1:12 AM · Restricted Project
jmolloy accepted D60982: [SimplifyCFG] Use lookup tables when they are more space efficient or a huge speed win..

This seems reasonable to me.

Thu, May 2, 1:12 AM · Restricted Project

Wed, May 1

jmolloy requested changes to D61160: [SimplifyCFG] Implement the suggested ctlz transform.
Wed, May 1, 1:58 AM · Restricted Project
jmolloy accepted D61151: [SimpligyCFG] NFC, remove GCD that was only used for powers of two.

LGTM.

Wed, May 1, 12:43 AM · Restricted Project

Tue, Apr 30

jmolloy accepted D61159: [SimplifyCFG] Run ReduceSwitchRange unconditionally, generalize.

LGTM, thanks for making the changes!

Tue, Apr 30, 8:48 AM · Restricted Project
jmolloy requested changes to D61158: [SimplifyCFG] use fshr instead of shl/lshr/or.

I don't agree; if you're going to add this canonicalization, please do it in InstCombine first.

Tue, Apr 30, 5:33 AM · Restricted Project
jmolloy requested changes to D61160: [SimplifyCFG] Implement the suggested ctlz transform.
Tue, Apr 30, 5:33 AM · Restricted Project
jmolloy requested changes to D61159: [SimplifyCFG] Run ReduceSwitchRange unconditionally, generalize.
Tue, Apr 30, 5:33 AM · Restricted Project
jmolloy requested changes to D61151: [SimpligyCFG] NFC, remove GCD that was only used for powers of two.
Tue, Apr 30, 4:32 AM · Restricted Project

Mon, Apr 29

jmolloy accepted D61237: [SimplifyCFG] ReduceSwitchRange: Improve on the case where the SubThreshold doesn't trigger.

LGTM with a couple of nits. Thanks!

Mon, Apr 29, 1:22 AM · Restricted Project
jmolloy added a comment to D61159: [SimplifyCFG] Run ReduceSwitchRange unconditionally, generalize.

Thanks for the changes, just two comments!

Mon, Apr 29, 1:10 AM · Restricted Project
jmolloy added inline comments to D61158: [SimplifyCFG] use fshr instead of shl/lshr/or.
Mon, Apr 29, 1:07 AM · Restricted Project
jmolloy added inline comments to D61151: [SimpligyCFG] NFC, remove GCD that was only used for powers of two.
Mon, Apr 29, 1:07 AM · Restricted Project

Apr 26 2019

jmolloy added inline comments to D61160: [SimplifyCFG] Implement the suggested ctlz transform.
Apr 26 2019, 6:10 AM · Restricted Project
jmolloy requested changes to D61160: [SimplifyCFG] Implement the suggested ctlz transform.

Thanks for implementing this! It's really cool.

Apr 26 2019, 1:44 AM · Restricted Project
jmolloy requested changes to D61158: [SimplifyCFG] use fshr instead of shl/lshr/or.
Apr 26 2019, 1:15 AM · Restricted Project
jmolloy added a comment to D61158: [SimplifyCFG] use fshr instead of shl/lshr/or.

Yes, I also have the same concerns as Roman. If we don't canonicalize to fshr in instcombine, then we shouldn't canonicalize to fshr in simplifycfg. They're both canonicalization passes.

Apr 26 2019, 1:15 AM · Restricted Project
jmolloy requested changes to D61159: [SimplifyCFG] Run ReduceSwitchRange unconditionally, generalize.

Thanks! Much easier to review this time.

Apr 26 2019, 1:08 AM · Restricted Project
jmolloy requested changes to D61151: [SimpligyCFG] NFC, remove GCD that was only used for powers of two.

Thanks for splitting these up! They're really much easier to read.

Apr 26 2019, 12:46 AM · Restricted Project
jmolloy accepted D61150: [SimplifyCFG] NFC, update Switch tests to HEAD so I can see if my changes change anything.

LGTM.

Apr 26 2019, 12:39 AM · Restricted Project

Apr 25 2019

jmolloy added inline comments to D60982: [SimplifyCFG] Use lookup tables when they are more space efficient or a huge speed win..
Apr 25 2019, 12:06 PM · Restricted Project
jmolloy requested changes to D60982: [SimplifyCFG] Use lookup tables when they are more space efficient or a huge speed win..

Please reupload with full context as described in the developer's guide.

Apr 25 2019, 9:00 AM · Restricted Project
jmolloy resigned from D61132: [builtins] run-time support for sparse maps in llvm.

I am not the right person to review this change. I have never made changes in this directory and don't know enough about potential impact to LGTM.

Apr 25 2019, 8:44 AM · Restricted Project, Restricted Project
jmolloy requested changes to D60673: [SimplifyCFG] Improove and speed up ReduceSwitchRange.

Hi Shawn,

Apr 25 2019, 8:38 AM · Restricted Project

Apr 23 2019

jmolloy created D61019: [AggressiveAntiDepBreaker] Handle predicate operands better.
Apr 23 2019, 7:50 AM · Restricted Project
jmolloy accepted D60090: [ARM] Update check for CBZ in Ifcvt.

Seems reasonable to me.

Apr 23 2019, 3:51 AM · Restricted Project

Apr 19 2019

jmolloy committed rG9ad4cb3de47e: [PATCH] [MachineScheduler] Check pending instructions when an instruction is… (authored by jmolloy).
[PATCH] [MachineScheduler] Check pending instructions when an instruction is…
Apr 19 2019, 2:00 AM
jmolloy added a comment to D60861: [MachineScheduler] Check pending instructions when an instruction is scheduled.

Thanks Hal! r358743.

Apr 19 2019, 2:00 AM · Restricted Project

Apr 18 2019

jmolloy added a comment to D60861: [MachineScheduler] Check pending instructions when an instruction is scheduled.

Hi Hal,

Apr 18 2019, 12:14 PM · Restricted Project
jmolloy created D60861: [MachineScheduler] Check pending instructions when an instruction is scheduled.
Apr 18 2019, 3:42 AM · Restricted Project

Sep 18 2017

jmolloy requested changes to D35014: [X86] Improvement in CodeGen instruction selection for LEAs..
Sep 18 2017, 5:04 AM

Jun 28 2017

jmolloy accepted D34398: [ARM] Improve if-conversion for M-class CPUs without branch predictors.

Hi John,

Jun 28 2017, 3:11 AM

May 25 2017

jmolloy added a comment to D24805: [GVNSink] Initial GVNSink prototype.

Thanks Danny,

May 25 2017, 6:20 AM

May 23 2017

jmolloy added a comment to D33446: [ARM] Temporarily disable globals promotion to constant pools to prevent miscompilation.

+1 from me.

May 23 2017, 11:19 AM

May 22 2017

jmolloy updated the diff for D24805: [GVNSink] Initial GVNSink prototype.

Hi Danny,

May 22 2017, 6:41 AM
jmolloy closed D25804: Fix 24560: assembler does not share constant pool for same constants.

Re-writing here because phab didn't pick up my email reply.

May 22 2017, 2:44 AM
jmolloy reopened D25804: Fix 24560: assembler does not share constant pool for same constants.

PR32825 seems valid and has been open for some time. Weiming, I have reverted this in r303535.

May 22 2017, 1:43 AM
jmolloy added a comment to D33186: [InstCombine] Canonicalize clamp of float types to minmax in fast mode..

And stating that they are would be incorrect.

May 22 2017, 1:22 AM

May 19 2017

jmolloy added a comment to D24805: [GVNSink] Initial GVNSink prototype.

No it won't, in the same way pre-order is not equivalent to predecessors before successors :)

May 19 2017, 7:34 AM
jmolloy updated the diff for D24805: [GVNSink] Initial GVNSink prototype.

Thanks for the review comments:

May 19 2017, 2:36 AM

May 18 2017

jmolloy added a reviewer for D24805: [GVNSink] Initial GVNSink prototype: eli.friedman.
May 18 2017, 8:22 AM
jmolloy updated the diff for D24805: [GVNSink] Initial GVNSink prototype.
  • Patch updated
  • Rebased onto NewGVN
  • No known defects - LNT, spec2k, spec2k6, usual suspects run, no problems.
  • All of the defects raised in SimplifyCFG's implementation have been added to the regression test suite for GVNSink.
May 18 2017, 8:22 AM

May 15 2017

jmolloy added a comment to D33186: [InstCombine] Canonicalize clamp of float types to minmax in fast mode..

Right; that's X86's strange not-a-maxnum-not-a-maxnan behaviour where it just selects the zeroth operand.

May 15 2017, 5:11 AM
jmolloy added a comment to D33186: [InstCombine] Canonicalize clamp of float types to minmax in fast mode..

On the other hand, some backends (e.g. X86) have only

FMIN/FMAX nodes that do not care about NaNS and the former NAN/NUM
nodes are illegal thus selection is not happening.

May 15 2017, 4:38 AM
jmolloy accepted D32857: [ARM] Mark LEApcrel as not having side effects.

LGTM

May 15 2017, 4:00 AM
jmolloy accepted D32858: [ARM] Mark LEApcrel instructions as isAsCheapAsAMove.

LGTM

May 15 2017, 3:59 AM
jmolloy added a comment to D33150: [WIP]Forming branch from select aggressively in loop.

Hi Jun,

May 15 2017, 12:41 AM

May 8 2017

jmolloy accepted D32847: [ARM] Clear the constant pool cache on explicit .ltorg directives.

I think this looks fine to me.

May 8 2017, 1:56 AM
jmolloy accepted D32938: [ARM][NEON] Add support for ISD::ABS lowering .

LGTM, thanks!

May 8 2017, 12:48 AM
jmolloy accepted D32940: [AARCH64][NEON] Add support for ISD::ABS lowering .

LGTM, thanks!

May 8 2017, 12:47 AM

Apr 28 2017

jmolloy accepted D32622: ARM: Compute MaxCallFrame size early.

OK, LGTM!

Apr 28 2017, 9:56 AM
jmolloy added inline comments to D32622: ARM: Compute MaxCallFrame size early.
Apr 28 2017, 12:28 AM

Apr 27 2017

jmolloy accepted D32551: [GlobalOpt] Correctly update metadata when localizing a global..

Ah, thanks Eli!

Apr 27 2017, 12:59 AM

Apr 25 2017

jmolloy accepted D31861: [Lexicon] Add BDCE.
Apr 25 2017, 6:07 AM
jmolloy accepted D32438: [LNT] Document how to capture linux perf profiles..
Apr 25 2017, 6:07 AM

Apr 21 2017

jmolloy added inline comments to D32323: [SimplifyCFG] Fix the determination of PostBB in conditional store merging to handle the targets on the second branch being commuted.
Apr 21 2017, 12:05 AM
jmolloy accepted D32323: [SimplifyCFG] Fix the determination of PostBB in conditional store merging to handle the targets on the second branch being commuted.

LGTM, thanks! Looks like the rest of the code was written with this in mind, but I never got around to it.

Apr 21 2017, 12:04 AM

Apr 20 2017

jmolloy accepted D32250: [Thumb-1] Fix corner cases for compressed jump tables.

LGTM, thanks for catching these!

Apr 20 2017, 12:52 AM

Apr 18 2017

jmolloy accepted D32163: [ARM,AArch64] Define __ELF__ for arm-none-eabihf and AArch64.

LGTM!

Apr 18 2017, 6:20 AM
jmolloy resigned from D32009: Allow attributes with global variables.

It looks good to me, but it would as I wrote this particular patch originally :) For that reason I can't effectively review it, so I'll resign.

Apr 18 2017, 1:33 AM
jmolloy accepted D32127: ARM: Use methods to access data stored with frame instructions.

One change requested, otherwise LGTM. Thanks for the cleanup!

Apr 18 2017, 1:28 AM

Apr 11 2017

jmolloy accepted D31933: [ARM] Refactor Thumb2 sat instructions.

LGTM, thanks!

Apr 11 2017, 7:38 AM
jmolloy accepted D31857: [Analysis] Support bitreverse in -demanded-bits pass.

LGTM, thanks!

Apr 11 2017, 12:17 AM

Apr 10 2017

jmolloy added a comment to D31081: [ARM] ScheduleDAGRRList::DelayForLiveRegsBottomUp must consider OptionalDefs.

I've spent some time coming up to speed on this in picking it up from Artyom. The million dollar question on this seems to be whether OptionalDefs are a total hack and should be ripped out entirely, or should be kept. I think in the latter case, the patch itself looks correct.

Apr 10 2017, 3:48 AM

Apr 7 2017

jmolloy accepted D31797: [ARM] Prefer BIC over BFC in ARM mode..
Apr 7 2017, 12:10 AM
jmolloy accepted D31795: [ARM] Remove redundant computeKnownBits helper..

Nice cleanup, thanks!

Apr 7 2017, 12:10 AM
jmolloy accepted D31793: [AArch64] Allow global register asm("x18") or asm("w18") under -ffixed-x18.

LGTM, thanks!

Apr 7 2017, 12:08 AM

Apr 6 2017

jmolloy accepted D31676: [SelectionDAG] [ARM CodeGen] Fix chain information of LowerMUL.

It took me a while to grok this, so here's some ascii-art for the record :)

Apr 6 2017, 2:08 AM

Apr 5 2017

jmolloy closed D31655: [LAA] Correctly return a half-open range in expandBounds.

r299526, thanks!

Apr 5 2017, 2:39 AM
jmolloy added a comment to D31625: [SDAG] Fix CombineTo ordering in visitZERO_EXTEND and visitSIGN_EXTEND.

Barring comments, my default plan is to mark the shared subset of instructions across the RUN calls that make sense.

Apr 5 2017, 12:57 AM
jmolloy requested changes to D31676: [SelectionDAG] [ARM CodeGen] Fix chain information of LowerMUL.

Thanks for this fix. I'm not 100% sure that all of it is needed - comments inline. Also, I particularly like the thoroughness of the commenting in your testcases. Much appreciated! :)

Apr 5 2017, 12:52 AM

Apr 4 2017

jmolloy added a comment to D31625: [SDAG] Fix CombineTo ordering in visitZERO_EXTEND and visitSIGN_EXTEND.

You're still checking the entire codegen output for this function. That's not what this test is about; this doesn't add any value, all it does is add maintenance burden.

Apr 4 2017, 8:01 AM
jmolloy added a comment to D31625: [SDAG] Fix CombineTo ordering in visitZERO_EXTEND and visitSIGN_EXTEND.

Hi Nirav,

Apr 4 2017, 7:48 AM
jmolloy added a comment to D31625: [SDAG] Fix CombineTo ordering in visitZERO_EXTEND and visitSIGN_EXTEND.

Those testcases are too strict. If you just need to check that a testcase doesn't cause an assertion failure, you can avoid any FileCheck checks and put "; REQUIRES: asserts" (I think that's the syntax) to ensure it only gets run on an asserts build.

Apr 4 2017, 7:18 AM
jmolloy added inline comments to D31655: [LAA] Correctly return a half-open range in expandBounds.
Apr 4 2017, 4:36 AM
jmolloy created D31655: [LAA] Correctly return a half-open range in expandBounds.
Apr 4 2017, 3:24 AM
jmolloy requested changes to D31625: [SDAG] Fix CombineTo ordering in visitZERO_EXTEND and visitSIGN_EXTEND.

Thanks for submitting this change, but I don't understand it. The phab issue title and summary don't seem to match up, the summary isn't descriptive enough for me to grok what's going on here and you haven't provided a testcase.

Apr 4 2017, 12:08 AM

Mar 31 2017

jmolloy added a comment to D31500: [ARM]Add a NOP instruction to make LLVM generate a mapping symbol.

I'm confused about how this test change is actually checking for any change in behaviour. No CHECK lines have been updated - have I missed something? Is the test currently failing?

Mar 31 2017, 12:26 AM

Mar 23 2017

jmolloy accepted D31253: [ARM] Delete duplicate known bits implementation for ARMISD::CMOV..

Looks even better than Pirama's patch. Thanks!

Mar 23 2017, 2:34 AM
jmolloy accepted D31265: Fix computeKnownBits for ARMISD::CMOV.

Ouch! LGTM, thanks!

Mar 23 2017, 2:28 AM

Mar 18 2017

jmolloy accepted D31116: [ARM] handle promotion of zero sized constants..

LGTM, thanks!

Mar 18 2017, 4:52 AM
jmolloy added a comment to D31116: [ARM] handle promotion of zero sized constants..

Hi Tim,

Mar 18 2017, 3:00 AM
jmolloy requested changes to D31116: [ARM] handle promotion of zero sized constants..

Hi Tim,

Mar 18 2017, 2:35 AM

Mar 15 2017

jmolloy accepted D30689: [ConstantFolding] Small fix to prevent constant folding having to repeatedly scan operands..

Hi David,

Mar 15 2017, 1:53 AM

Mar 14 2017

jmolloy accepted D30781: [ValueTracking] Out of range shifts might be undef.

Took a bit of staring to convince myself it was right, but LGTM.

Mar 14 2017, 2:40 AM
jmolloy added a comment to D30689: [ConstantFolding] Small fix to prevent constant folding having to repeatedly scan operands..

The original code looks significantly dodgy to me. From a read of the code and the intent, I think your change is right (the original code always added {FoldedC, FoldedC} which is totally bogus).

Mar 14 2017, 1:54 AM

Mar 10 2017

jmolloy accepted D30782: imm_comp_XFORM (defined in ARMInstrThumb.td) duplicates imm_not_XFORM (defined in ARMInstrInfo.td).

Nice cleanup, thanks!

Mar 10 2017, 3:03 AM
jmolloy added a comment to D30333: Split SimplifyCFG to run obscuring switch transforms only during last phase.

Looks good on my side, but please wait for Mehdi.

Mar 10 2017, 2:17 AM
jmolloy accepted D30794: [ARM] Replace some C++ selection code with TableGen patterns. NFC..

Nice cleanup! LGTM

Mar 10 2017, 1:14 AM
jmolloy accepted D30116: Refactor SimplifyCFG:canSinkInstructions [NFC].

LGTM

Mar 10 2017, 1:00 AM

Mar 6 2017

jmolloy added a comment to D30609: [SimplifyCFG] do not sink intrinsics even with non-constant operands.

For me, that sounds OK. I'd like @arsenm to confirm my understanding about the convergent attribute though.

Mar 6 2017, 8:31 AM

Mar 5 2017

jmolloy added a comment to D30609: [SimplifyCFG] do not sink intrinsics even with non-constant operands.

Hi Matt,

Mar 5 2017, 11:54 PM
jmolloy requested changes to D30609: [SimplifyCFG] do not sink intrinsics even with non-constant operands.

I'm not sure this is the best approach. What you're essentially saying here is that we should avoid sinking all intrinsics because they may have unmodelled sideeffects. What I'd prefer is for us to understand which intrinsics have such sideeffects and special case them.

Mar 5 2017, 7:31 AM