Page MenuHomePhabricator

joanlluch (Joan LLuch)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 9 2019, 2:05 AM (31 w, 11 h)

Recent Activity

Yesterday

joanlluch updated the summary of D70042: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (4).
Mon, Nov 11, 10:04 AM · Restricted Project
joanlluch added inline comments to D70042: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (4).
Mon, Nov 11, 8:22 AM · Restricted Project
joanlluch updated the diff for D70042: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (4).
Mon, Nov 11, 8:13 AM · Restricted Project
joanlluch added inline comments to D70042: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (4).
Mon, Nov 11, 7:36 AM · Restricted Project
joanlluch committed rGabbbf9880c27: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (4)… (authored by joanlluch).
[TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (4)…
Mon, Nov 11, 7:18 AM
joanlluch closed D70083: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (4) (Baseline tests).
Mon, Nov 11, 7:18 AM · Restricted Project
joanlluch created D70083: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (4) (Baseline tests).
Mon, Nov 11, 7:18 AM · Restricted Project
joanlluch committed rGe0012c5d6acb: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3) (authored by joanlluch).
[TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3)
Mon, Nov 11, 1:20 AM
joanlluch closed D69326: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3).
Mon, Nov 11, 1:20 AM · Restricted Project
joanlluch updated the summary of D69326: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3).
Mon, Nov 11, 1:10 AM · Restricted Project

Sat, Nov 9

joanlluch created D70042: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (4).
Sat, Nov 9, 2:47 AM · Restricted Project

Fri, Nov 8

joanlluch updated the diff for D69326: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3).

Updated after D69975 was reviewed and committed.
Now showing the codegen improvements related with this patch

Fri, Nov 8, 11:55 PM · Restricted Project
joanlluch added a comment to D69975: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3) (baseline tests).

This is now commited to the github repo
https://github.com/llvm/llvm-project/commit/fe0763d28a572f72007637c7bd097bc19cbb58fc

Fri, Nov 8, 2:33 PM · Restricted Project
joanlluch committed rGfe0763d28a57: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3)… (authored by joanlluch).
[TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3)…
Fri, Nov 8, 2:25 PM
joanlluch closed D69975: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3) (baseline tests).
Fri, Nov 8, 2:24 PM · Restricted Project
joanlluch added a comment to D69975: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3) (baseline tests).

Hi @spatel, @asl
Thanks for reviewing this. I'm getting a bit embarrased to haveto ask the following, but I don't seem to figure out a way to commit this using 'git' alone. I would appreciate some help.
I currently have a local 'master' branch that I keep identical to the upstream branch. I also have another local branch (let's call it 'patchbranch' for the purposes of this) where I committed the changes for this patch. The diff file is created by comparing 'master' with 'patchbranch' and uploaded here. So far so good. To make sure my 'patchbranch' catches all the upstream changes I pull from 'master', merge 'master' into my 'patchbranch and run the tests again. Now I want to push my local 'patchbranch' to the upstream 'master'. This is where I get stuck. I also need to make sure that this is pushed as a single commit, but I do not know how to achieve that either. I do not want to merge my local 'patchbranch' into my local 'master' because I want to keep my local 'master' clean and identical to the upstream branch. I have read the documentation but all the utilities and workflows seems to me all interleaved with svn which I do not want or know how to use.
Thanks in advance and sorry for that.

Fri, Nov 8, 2:03 AM · Restricted Project

Thu, Nov 7

joanlluch added a reviewer for D69975: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3) (baseline tests): lebedev.ri.
Thu, Nov 7, 3:50 PM · Restricted Project
joanlluch planned changes to D69326: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3).

Following the above conversation, I created D69975 as the base tests before this patch can be committed.
I put the base tests in a separate patch to actually show the differential effects of this one.
I will open this for review after D69975 is reviewed.
Thanks,

Thu, Nov 7, 3:50 PM · Restricted Project
joanlluch updated the diff for D69975: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3) (baseline tests).
Thu, Nov 7, 3:41 PM · Restricted Project
joanlluch added reviewers for D69975: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3) (baseline tests): spatel, asl.
Thu, Nov 7, 3:41 PM · Restricted Project
joanlluch created D69975: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3) (baseline tests).
Thu, Nov 7, 3:32 PM · Restricted Project
joanlluch added a comment to D69326: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3).

Usually, a first commit will fix a code comment with a spelling error or formatting problem and be "NFC" (no-functional-change). Feel free to try that to make sure things are working.

Thu, Nov 7, 9:42 AM · Restricted Project
joanlluch added a comment to D69326: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3).

You will want to rebase this patch either way - I had to do some cleaning just to step through this function:
rG2fdd58c5066f
rG777d1d1d9811

Ok, no problem.

Thu, Nov 7, 9:42 AM · Restricted Project
joanlluch committed rG0d3d3822f53b: comment shiftamountthreshold (authored by joanlluch).
comment shiftamountthreshold
Thu, Nov 7, 8:46 AM

Wed, Nov 6

joanlluch added a comment to D69326: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3).

@spatel Thanks for reviewing and commiting my previous patches on this subject. I have now been given commit access. I have not tried yet, but it should be good.

Wed, Nov 6, 4:25 PM · Restricted Project
joanlluch updated the diff for D69326: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3).

Made sure diff matches latest trunk changes

Wed, Nov 6, 3:19 PM · Restricted Project

Thu, Oct 31

joanlluch added a comment to D69120: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (2/2).

@asl Ok thanks, I agree that 9 bit shifts may also benefit from the version with amount.
If this is ok, I will propose the TLI function replacement on a different patch after this is reviewed, to keep things organised.

Thu, Oct 31, 11:16 AM · Restricted Project
joanlluch updated the diff for D69120: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (2/2).

Updated diff to account for the latest truck changes

Thu, Oct 31, 10:58 AM · Restricted Project

Wed, Oct 30

joanlluch added inline comments to D69609: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (Baseline).
Wed, Oct 30, 11:12 AM · Restricted Project
joanlluch added a comment to D69609: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (Baseline).

LGTM - have you requested/acquired commit privilege yet?

Wed, Oct 30, 9:21 AM · Restricted Project
joanlluch added inline comments to D69609: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (Baseline).
Wed, Oct 30, 9:12 AM · Restricted Project
joanlluch added a comment to D69120: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (2/2).

@asl I identified you as the creator of the MSP430 patch that optimised 8 bit and larger shifts by combining them with bswap and extend instructions. This creates relatively cheap instruction sequences for exactly 8 bit shifts.

Wed, Oct 30, 3:08 AM · Restricted Project
joanlluch updated the diff for D69120: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (2/2).

@spatel I updated this revision diff according to your suggestions. I also created a new baseline test revision to help identify the effects of this patch. I hope this is ok. Thanks in advance for your time.

Wed, Oct 30, 2:12 AM · Restricted Project
joanlluch added a comment to D69120: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (2/2).

I created a new baseline revision, D69609, that help to better reflect the effects of applying this patch. The referred revision should therefore be applied before this one for consistency reasons.

Wed, Oct 30, 2:03 AM · Restricted Project
joanlluch created D69609: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (Baseline).
Wed, Oct 30, 1:44 AM · Restricted Project

Tue, Oct 29

joanlluch planned changes to D69120: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (2/2).
Tue, Oct 29, 9:44 PM · Restricted Project
joanlluch added inline comments to D69120: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (2/2).
Tue, Oct 29, 2:40 PM · Restricted Project
joanlluch updated the diff for D69120: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (2/2).

Updated tests to reflect each covered case in an independent way with more focused IR patterns.
Added comments to the test file that refer to the transformation we are dealing with.

Tue, Oct 29, 2:23 PM · Restricted Project

Tue, Oct 22

joanlluch updated the summary of D69326: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3).
Tue, Oct 22, 11:34 PM · Restricted Project
joanlluch created D69326: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3).
Tue, Oct 22, 3:18 PM · Restricted Project
joanlluch added reviewers for D69326: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (3): spatel, lebedev.ri, asl.
Tue, Oct 22, 3:18 PM · Restricted Project
joanlluch updated the diff for D69120: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (2/2).

Code style correction

Tue, Oct 22, 4:20 AM · Restricted Project

Sun, Oct 20

joanlluch updated the diff for D69120: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (2/2).

Made sure the updated diff file accounts for recent trunk changes

Sun, Oct 20, 11:46 AM · Restricted Project

Sat, Oct 19

joanlluch updated the diff for D69116: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (1/2).

Move repeated calls to N0.getValueType() into a variable

Sat, Oct 19, 1:33 AM · Restricted Project

Fri, Oct 18

joanlluch added a comment to D69116: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (1/2).

LGTM - see inline for a minor improvement.

Do you have commit access?

Fri, Oct 18, 3:27 PM · Restricted Project
joanlluch added inline comments to D69116: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (1/2).
Fri, Oct 18, 3:27 PM · Restricted Project
joanlluch updated the diff for D69120: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (2/2).
Fri, Oct 18, 4:02 AM · Restricted Project
joanlluch updated the diff for D69116: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (1/2).

Updated with latest suggestions

Fri, Oct 18, 3:38 AM · Restricted Project

Thu, Oct 17

joanlluch added inline comments to D69116: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (1/2).
Thu, Oct 17, 3:06 PM · Restricted Project
joanlluch added inline comments to D69116: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (1/2).
Thu, Oct 17, 2:46 PM · Restricted Project
joanlluch added a reviewer for D69120: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (2/2): asl.
Thu, Oct 17, 12:26 PM · Restricted Project
joanlluch updated the diff for D69120: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (2/2).
Thu, Oct 17, 11:21 AM · Restricted Project
joanlluch abandoned D68982: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine.

Follow ups in D69099 , D69116 and D69120

Thu, Oct 17, 10:42 AM · Restricted Project
joanlluch created D69120: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (2/2).
Thu, Oct 17, 10:42 AM · Restricted Project
joanlluch created D69116: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (1/2).
Thu, Oct 17, 9:55 AM · Restricted Project
joanlluch updated the diff for D69099: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (Baseline Tests).

Should be ok now.

Thu, Oct 17, 8:49 AM · Restricted Project
joanlluch planned changes to D69099: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (Baseline Tests).

@spatel
I just realised that I somehow messed this test file with the already optimised one. Please discard this until I upload the right one. Thanks

Thu, Oct 17, 7:39 AM · Restricted Project
joanlluch updated the diff for D69099: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (Baseline Tests).

Updated baseline test file with the given recommendations:

  • removed explicit target and datalayout from .ll test file
  • removed dso_local
Thu, Oct 17, 7:12 AM · Restricted Project
joanlluch added inline comments to D69099: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (Baseline Tests).
Thu, Oct 17, 6:03 AM · Restricted Project
joanlluch created D69099: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (Baseline Tests).
Thu, Oct 17, 4:20 AM · Restricted Project

Wed, Oct 16

joanlluch added a comment to D68982: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine.

I know this is possibly not the right place to ask this particular question, but I thought that I would get a more focused reply by asking here:

Wed, Oct 16, 7:11 AM · Restricted Project
joanlluch added a comment to D68982: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine.
In D68982#1710606, @asl wrote:

@joanlluch There are two things here. We understand that there might be some out-of-tree targets that would benefit from changes in the mainline. However, it is still important to add tests that utilize mainline targets as this ensures that the proposed changes won't e.g. break during some refactoring, etc. So, using MSP430 (probably the smallest) / AVR is perfectly fine for tests, etc.!

Wed, Oct 16, 3:08 AM · Restricted Project
joanlluch added a comment to D68982: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine.
In D68982#1710529, @asl wrote:

The MSP430 changes / overall direction looks extremely useful to me. We just need to ensure everything is done properly. This is exactly what @spatel proposed.

I will be happy to review all MSP430 stuff :)

Hi,
Sorry, I somehow missed your post before I posted my last one. Thanks for that.

Wed, Oct 16, 2:54 AM · Restricted Project
joanlluch added a comment to D68982: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine.

Hi @spatel
Thank you for your support. I'll try to do what you suggest regarding generating of baseline tests and splitting the patch.

Wed, Oct 16, 2:20 AM · Restricted Project

Tue, Oct 15

joanlluch abandoned D68957: Shift Amount Threshold in DAGCombine.

I abandoned this, created D68982 instead

Tue, Oct 15, 4:33 AM · Restricted Project
joanlluch created D68982: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine.
Tue, Oct 15, 4:22 AM · Restricted Project

Mon, Oct 14

joanlluch updated the summary of D68957: Shift Amount Threshold in DAGCombine.
Mon, Oct 14, 3:46 PM · Restricted Project
joanlluch updated the summary of D68957: Shift Amount Threshold in DAGCombine.
Mon, Oct 14, 3:46 PM · Restricted Project
joanlluch updated the summary of D68957: Shift Amount Threshold in DAGCombine.
Mon, Oct 14, 3:41 PM · Restricted Project
joanlluch created D68957: Shift Amount Threshold in DAGCombine.
Mon, Oct 14, 3:28 PM · Restricted Project

Oct 13 2019

joanlluch added a comment to D63382: [InstCombine] fold a shifted zext to a select.

@spatel I want to express my support to selects over bit manipulation instructions in IR, as stated above, in order to move such optimisations to DAGCombine. Ideally, this should involve the removal of some of the existing InstCombineSelect transformations, particularly most of the ones in foldSelectInstWithICmp. However, as I exposed earlier in LLVM-dev, the DAGCombine code should incorporate hooks to allow targets to decide whether such bihacks are actually profitable, or it's best to keep them as selects.

Oct 13 2019, 12:18 PM · Restricted Project

Oct 12 2019

joanlluch added a comment to D63382: [InstCombine] fold a shifted zext to a select.

@joanlluch - this is a case where we could transform incoming shift to select and benefit small targets as you've noted.

Oct 12 2019, 11:31 AM · Restricted Project

Oct 3 2019

joanlluch accepted D68397: [DAGCombiner] add operation legality checks before creating shift ops (PR43542).

It works for me.

Oct 3 2019, 2:15 PM · Restricted Project

Jul 8 2019

joanlluch accepted D64300: Fix issues building libraries as more than one type with Xcode.

This fixed bug 42528. okey to me.

Jul 8 2019, 10:51 AM · Restricted Project
joanlluch abandoned D63692: [LSR] Improved code generation for Zero Compare loops.
Jul 8 2019, 8:26 AM · Restricted Project

Jun 27 2019

joanlluch added a comment to D63692: [LSR] Improved code generation for Zero Compare loops.

@craig.topper , can this be considered ok for the X86?. Thanks.

Jun 27 2019, 1:28 AM · Restricted Project

Jun 26 2019

joanlluch added a comment to D63692: [LSR] Improved code generation for Zero Compare loops.

Hi Craig, thanks for commenting.
Yes, I was actually compiling for -Oz, but the differences when using -Os are even bigger. Let me try to explain every case.

  • For Oz, the compiler indeed generates expensive push/pop instructions as an attempt to reduce code size. However, after the patch is applied these instructions are removed without increasing code size. I have checked that in several scenarios and the result is either the same size or less size. This is because the patch reduces the overall number of instructions. On the examples above, the resulting code size is identical because there's the fo and it's even reduced in some cases. This is because the sequence:
Jun 26 2019, 2:11 AM · Restricted Project
joanlluch added a comment to D63692: [LSR] Improved code generation for Zero Compare loops.

Hi shchenz,

Jun 26 2019, 12:57 AM · Restricted Project
joanlluch updated the diff for D63692: [LSR] Improved code generation for Zero Compare loops.

Fixes reported assertion crash on SystemZ

Jun 26 2019, 12:15 AM · Restricted Project

Jun 25 2019

joanlluch added a comment to D63477: [PowerPC] exclude ICmpZero Use in LSR if icmp can be replaced inside hardware loop..

I recently sent a new LSR patch proposal, D63692, that to some extent runs in the opposite direction than this one. I mean, I am adding an additional initial formula for ICmpZero as opposed to excluding it. I think that both patches are however fully compatible and probably worth merging together to get the best of all platforms. I added @shchenz, the author of this, as a D63692 reviewer so he can post any considerations.

Jun 25 2019, 12:47 PM · Restricted Project
joanlluch added a reviewer for D63692: [LSR] Improved code generation for Zero Compare loops: shchenz.
Jun 25 2019, 4:51 AM · Restricted Project
joanlluch added a reviewer for D63692: [LSR] Improved code generation for Zero Compare loops: javed.absar.
Jun 25 2019, 12:34 AM · Restricted Project
joanlluch changed the repository for D63692: [LSR] Improved code generation for Zero Compare loops from rL LLVM to rG LLVM Github Monorepo.
Jun 25 2019, 12:34 AM · Restricted Project

Jun 23 2019

joanlluch created D63692: [LSR] Improved code generation for Zero Compare loops.
Jun 23 2019, 10:25 AM · Restricted Project