andreadb (Andrea Di Biagio)
User

Projects

User does not belong to any projects.

User Details

User Since
May 9 2013, 11:10 AM (232 w, 3 d)

Recent Activity

Today

andreadb accepted D39169: [X86][SSE] Remove AssertZext stage from PEXTRW/PEXTRB lowering. NFCI..

Lgtm.

Mon, Oct 23, 7:39 AM
andreadb accepted D38696: [DAGCombine] Permit combining of shuffle of equivalent splat BUILD_VECTORs.

LGTM.
Thanks Simon!

Mon, Oct 23, 6:17 AM

Thu, Oct 12

andreadb added inline comments to D38696: [DAGCombine] Permit combining of shuffle of equivalent splat BUILD_VECTORs.
Thu, Oct 12, 11:44 AM

Aug 29 2017

andreadb updated subscribers of D36617: AMD Zen Scheduler Model Update.

I noticed that you forgot to include llvm-commits as a subscriber.

Aug 29 2017, 4:45 AM

May 12 2017

andreadb accepted D32563: Add LiveRangeShrink pass to shrink live range within BB..

Thanks for fixing the issue with debug values.

May 12 2017, 3:55 AM

May 11 2017

andreadb accepted D32563: Add LiveRangeShrink pass to shrink live range within BB..

Accepting this now as it looks reasonably safe and fast to me (nitpicks below).

It is a conservative/simple heuristic but given the fact that we lack other means of global code motion (in the sense of inter-basic block) in CodeGen today this is fine.

Please wait for some of the other reviewers which showed interest here to accept before committing.

May 11 2017, 11:09 AM
andreadb added a comment to D32563: Add LiveRangeShrink pass to shrink live range within BB..

Currently, we set 'SawStore' if there is at least one 'mayStore' instruction in the scheduling region.

May 11 2017, 6:26 AM

May 10 2017

andreadb added inline comments to D32563: Add LiveRangeShrink pass to shrink live range within BB..
May 10 2017, 12:22 PM

May 9 2017

andreadb added inline comments to D32563: Add LiveRangeShrink pass to shrink live range within BB..
May 9 2017, 5:55 AM

May 8 2017

andreadb accepted D32770: [X86][LWP] Add clang support for LWP instructions..

LGTM.

May 8 2017, 3:52 AM

May 5 2017

andreadb added a comment to D32563: Add LiveRangeShrink pass to shrink live range within BB..

Sorry, did not address this in the last patch. Done now. Note that I do not want to touch unnecessary files in this change. So I simply copied the code from MachineSink.cpp, and will refactor it to a utility function in MachineInstr.h/.cpp in a follow-up patch.

May 5 2017, 11:52 AM
andreadb added a comment to D32563: Add LiveRangeShrink pass to shrink live range within BB..

When you move an instruction, you should also move its associated debug values.

May 5 2017, 10:52 AM
andreadb added a comment to D32563: Add LiveRangeShrink pass to shrink live range within BB..

I had a quick look at the code, and I noticed that you never check for the presence of DBG_VALUE instructions in a basic block.
I think that your pass would not work as expected with debug info.

May 5 2017, 8:35 AM

May 3 2017

andreadb accepted D32769: [X86][LWP] Add llvm support for LWP instructions..

LGTM. Thanks!

May 3 2017, 6:59 AM
andreadb added a comment to D32769: [X86][LWP] Add llvm support for LWP instructions..

I noticed that according to X86GenInstrInfo.inc, the new LWPINS instructions don't have "UnmodeledSideEffects".
However, all the other LWP instructions have flag UnmodeledSideEffects set.
My understanding is that this is because LWPVAL/LLWPCB/SLWPCB are directly matched from x86 intrinsics with side effects, while LWPINS* are firstly lowered to X86ISD::LWPINS nodes.

May 3 2017, 4:11 AM

May 1 2017

andreadb added a comment to D32563: Add LiveRangeShrink pass to shrink live range within BB..

In all the new test cases, the problem is indirectly caused by the presence of function calls in the reassociated expression.
Your patch is essentially trying to solve a register pressure problem at LLVM IR level by pre-scheduling instructions. However, as Sanjay pointed out, this design can be a bit fragile since later passes may undo your canonicalization.

May 1 2017, 4:26 AM

Apr 20 2017

andreadb accepted D32283: [X86] Remove dead code. .

LGTM. Thanks!

Apr 20 2017, 8:55 AM
andreadb accepted D32002: [X86] Improve large struct pass by value performance.

LGTM too.

Apr 20 2017, 8:53 AM

Apr 19 2017

andreadb added a comment to D32236: PR32710: Disable using PMADDWD for unsigned short..

Hi Dehao (and Michael),

Apr 19 2017, 11:58 AM
andreadb accepted D31960: [InstSimplify] fold identity shuffles (recursing if needed).

I only have one request. Otherwise it LGTM too.

Apr 19 2017, 6:30 AM

Apr 18 2017

andreadb added a comment to D32002: [X86] Improve large struct pass by value performance.

There are two sides to this flag:

Using REPMOVSB instead of REPMOVSQ: When this flag is true, then the code suggested in the PR is always more efficient regardless of the size.
Deciding whether to use REPMOVS instead of chains of mov/vmovups/... (which are handled in a generic manner by getMemcpyLoadsAndStores() in CodeGen/SelectionDAG/SelectionDAG.cpp).
Apr 18 2017, 6:50 AM

Apr 13 2017

andreadb created D32008: [SampleProfile] Skip intrinsic calls when visiting callsites in InlineHotFunctions. NFC..
Apr 13 2017, 5:02 AM

Apr 11 2017

andreadb updated the diff for D31900: [AddDiscriminators] Assign discriminators to memset/memcpy/memmove intrinsic calls..

Rebase and address code review comments.

Apr 11 2017, 11:24 AM
andreadb added a comment to D31900: [AddDiscriminators] Assign discriminators to memset/memcpy/memmove intrinsic calls..

Thanks for the explanation. This makes sense. The reasons that we try to avoid IntrinsicInst is 2 fold:

  1. avoid non-deterministic discriminator assignment for different debug level
  2. minimize the number of base discriminators

    Apparently, bypassing all IntrinsicInst is an overkill to solve #1, and in fact it will cause the bug as shown in the unittest, and your patch managed to fix it. But for #2, your patch introduces extra base discriminators that is unnecessary. I suggest only keep the first callsite of shouldHaveDiscriminator, and revert the 2nd callsite to just check IntrinsicInst. Please add comment at both places to explain what's the motivation (the first place, i.e. call to shouldHaveDiscriminator is aiming at addressing #1, the 2nd place, i.e. checking IntrinsicInst, is aiming at addressing #1 and #2). And also in the unittest, please check explicitly that the discriminator for the memcpy intrinsic is the same as other instructions in that BB.

Thanks for the feedback.
I will add the comments you have requested and I will restore the 2nd callsite to the check for IntrinsicInst.

Do you want me to keep test memcpy-discriminator.ll in the final patch?

About the unit test: is there a template that I can use for this particular case? I never had to write an llvm unittest before, so I don't know where to look at for examples.

Thanks,
Andrea

Apr 11 2017, 9:36 AM
andreadb added a comment to D31900: [AddDiscriminators] Assign discriminators to memset/memcpy/memmove intrinsic calls..

Thanks for the explanation. This makes sense. The reasons that we try to avoid IntrinsicInst is 2 fold:

  1. avoid non-deterministic discriminator assignment for different debug level
  2. minimize the number of base discriminators

    Apparently, bypassing all IntrinsicInst is an overkill to solve #1, and in fact it will cause the bug as shown in the unittest, and your patch managed to fix it. But for #2, your patch introduces extra base discriminators that is unnecessary. I suggest only keep the first callsite of shouldHaveDiscriminator, and revert the 2nd callsite to just check IntrinsicInst. Please add comment at both places to explain what's the motivation (the first place, i.e. call to shouldHaveDiscriminator is aiming at addressing #1, the 2nd place, i.e. checking IntrinsicInst, is aiming at addressing #1 and #2). And also in the unittest, please check explicitly that the discriminator for the memcpy intrinsic is the same as other instructions in that BB.
Apr 11 2017, 9:28 AM
andreadb added a comment to D31900: [AddDiscriminators] Assign discriminators to memset/memcpy/memmove intrinsic calls..

Thanks for explaining. My understanding is that memcpy will introduce new basic block, which will share the same discriminator with other basic block without this patch. As a result, that basic block is incorrectly annotated so that the block placement is suboptimal. If my understanding is correct, could you include a that in the unittest?

Apr 11 2017, 8:22 AM
andreadb added a comment to D31900: [AddDiscriminators] Assign discriminators to memset/memcpy/memmove intrinsic calls..

I don't quite understand why you want to set a new discriminator for memcpy builtin. What optimization would it enable? For normal function calls, we need to have discriminator to distinguish callsites in the same BB so that we can annotate the inlined callee correctly. But for the memcpy case, looks like adding a new discriminator does not help down-stream optimizations like inlining?

Apr 11 2017, 4:12 AM

Apr 10 2017

andreadb created D31900: [AddDiscriminators] Assign discriminators to memset/memcpy/memmove intrinsic calls..
Apr 10 2017, 11:33 AM

Mar 21 2017

andreadb added a comment to D30835: [DebugInfo][X86] Teach Optimize LEAs pass to handle debug values.

I am going to commit this on Andrew's behalf.

Mar 21 2017, 4:47 AM

Mar 13 2017

andreadb accepted D30833: [X86][MMX] Fix folding of shift value loads to cover whole 64-bits.

Looks good to me.

Mar 13 2017, 11:33 AM

Mar 2 2017

andreadb accepted D29813: [DAGCombiner] Fix DebugLoc propagation when folding !(x cc y) -> (x !cc y).

LGTM too!

Mar 2 2017, 4:43 AM

Feb 24 2017

andreadb added a comment to D30226: [BranchFolding] Merge debug locations from common tail instead of removing.
In D30226#685947, @twoh wrote:

Yes I observed the case from refresh_potential function in spec2006 429.mcf. The original C code snippet is below:

 81     while( node != root )
 82     {
 83         while( node )
 84         {
 85             if( node->orientation == UP )
 86                 node->potential = node->basic_arc->cost + node->pred->potential;
 87             else /* == DOWN */
 88             {
 89                 node->potential = node->pred->potential - node->basic_arc->cost;
 90                 checksum++;
 91             }
...

Before Control Flow Optimizer pass (which runs BranchFolder pass), instructions for line 83 are located in two places, one in the loop preheader and the other in the loop body:

BB#4: derived from LLVM BB %while.cond3.preheader
  TEST64rr %RDX, %RDX, %EFLAGS<imp-def>; dbg:mcfutil.c:83:9
  JE_1 <BB#5>, %EFLAGS<imp-use>; dbg:mcfutil.c:83:9
  JMP_1 <BB#6>; dbg:mcfutil.c:83:9

...

BB#6: derived from LLVM BB %while.body4
...
  TEST64rr %RDX, %RDX, %EFLAGS<imp-def>; dbg:mcfutil.c:83:9
  JE_1 <BB#5>, %EFLAGS<imp-use>; dbg:mcfutil.c:83:9
  JMP_1 <BB#6>; dbg:mcfutil.c:83:9

And these two blocks are tail merged by BranchFolder pass, but without this patch, debug locations are gone in the common tail.

Feb 24 2017, 11:13 AM
andreadb added a comment to D30226: [BranchFolding] Merge debug locations from common tail instead of removing.

However, if identical insturctions that are merged into a common tail have the same debug locations, there's no need to remove them.

Feb 24 2017, 8:24 AM

Feb 22 2017

andreadb added inline comments to D29813: [DAGCombiner] Fix DebugLoc propagation when folding !(x cc y) -> (x !cc y).
Feb 22 2017, 9:19 AM
andreadb added inline comments to D29813: [DAGCombiner] Fix DebugLoc propagation when folding !(x cc y) -> (x !cc y).
Feb 22 2017, 4:44 AM

Feb 21 2017

andreadb added inline comments to D30213: [X86][SSE] Enable post-legalize vXi64 shuffle combining on 32-bit targets.
Feb 21 2017, 10:57 AM
andreadb added a reviewer for D29813: [DAGCombiner] Fix DebugLoc propagation when folding !(x cc y) -> (x !cc y): aprantl.
Feb 21 2017, 5:09 AM
andreadb added inline comments to D29813: [DAGCombiner] Fix DebugLoc propagation when folding !(x cc y) -> (x !cc y).
Feb 21 2017, 5:08 AM

Feb 16 2017

andreadb added a comment to D29959: x86 interrupt calling convention: only save xmm registers if the target supports SSE.

Could someone commit this for me, please?

Feb 16 2017, 10:35 AM
andreadb accepted D29959: x86 interrupt calling convention: only save xmm registers if the target supports SSE.
Feb 16 2017, 3:52 AM

Feb 15 2017

andreadb added a comment to D29959: x86 interrupt calling convention: only save xmm registers if the target supports SSE.

I only have a minor comment about the test.

Feb 15 2017, 9:06 AM
andreadb accepted D29961: [DAG] Don't try to create an INSERT_SUBVECTOR with an illegal source.

Thanks for the quick fix.

Feb 15 2017, 3:57 AM

Feb 14 2017

andreadb added a comment to D29927: [DebugInfo] Make DILocation::getMergedLocation to return a dummy location instead of nullptr.

FYI. An alternative fix for this same issue has been already uploaded by Adrian as review D29833.

Feb 14 2017, 6:07 AM

Feb 10 2017

andreadb accepted D29820: [X86] Improve readability of test/CodeGen/X86/lzcnt-zext-cmp.ll by adding a common check prefix ALL..

LGTM

Feb 10 2017, 5:20 AM

Feb 9 2017

andreadb accepted D28621: X86: Teach X86InstrInfo::analyzeCompare to recognize compares of symbols..

Looks good to me too.

Feb 9 2017, 12:22 PM
andreadb accepted D29756: [X86][btver2] PR31902: Fix a crash in combineOrCmpEqZeroToCtlzSrl under fast math..

Basically, there is no guarantee that N->getOperand(1) is an integer value.

Feb 9 2017, 4:07 AM

Feb 8 2017

andreadb added a comment to D29385: Clzero intrinsic and its addition under znver1.

The instruction will default to having MCID::UnmodeledSideEffects set. Is that sufficient to protect the scheduling?

Feb 8 2017, 4:41 AM

Feb 7 2017

andreadb added a comment to D29385: Clzero intrinsic and its addition under znver1.

Ah, I see that Simon asked a similar question :-)

Feb 7 2017, 9:39 AM
andreadb added a comment to D29385: Clzero intrinsic and its addition under znver1.

I have a (probably dumb) question: should CLZERO be treated as a memory read/write barrier for the purpose of scheduling? Is it okay to reoder CLFLUSH based on the EAX/RAX register dependency only?

Feb 7 2017, 9:38 AM

Feb 6 2017

andreadb added a reviewer for D29504: Merge DebugLoc on combined stores: aprantl.
Feb 6 2017, 4:59 AM
andreadb added inline comments to D29504: Merge DebugLoc on combined stores.
Feb 6 2017, 4:52 AM
andreadb accepted D29399: [X86][SSE] Combine shuffle nodes with multiple uses if all the users are being combined..

Hi Simon,
thanks for the extra tests.

Feb 6 2017, 4:09 AM

Feb 1 2017

andreadb added a comment to D29399: [X86][SSE] Combine shuffle nodes with multiple uses if all the users are being combined..

Hi Simon,

Feb 1 2017, 12:51 PM
andreadb accepted D28810: [X86][SSE] Use MOVMSK for all_of/any_of reduction patterns.

To avoid the partial register issue, I've updated the result select code to always perform it as i32/i64 followed by a truncation if necessary.

Feb 1 2017, 10:55 AM
andreadb added inline comments to D28810: [X86][SSE] Use MOVMSK for all_of/any_of reduction patterns.
Feb 1 2017, 10:36 AM
andreadb added inline comments to D28810: [X86][SSE] Use MOVMSK for all_of/any_of reduction patterns.
Feb 1 2017, 8:20 AM
andreadb added inline comments to D28810: [X86][SSE] Use MOVMSK for all_of/any_of reduction patterns.
Feb 1 2017, 5:21 AM

Jan 31 2017

andreadb accepted D29254: Do not propagate DebugLoc across basic blocks.

This patch looks good to me.

Jan 31 2017, 12:38 PM

Jan 27 2017

andreadb added a comment to D29088: Do not create ctlz/cttz(X, false) when the target do not support zero defined ctlz/cttz..

I agree that we are currently missing an optimization.
That said, (if I remember correctly) the only place where we form cttz/ctlz with is_zero_undef=false is in foldSelectCttzCtlz() and the only goal of that transform is to canonicalize cttz/ctlz in preparation for codegen. That's why I suggested considering the possibility of moving that transform into CGP. If we do this, then we no longer need to add extra optimization rules to "fix" the fact that we prematurely canonicalized.

We also are doing it in InstCombine ( see foldCttzCtlz ) using isKnownNonZero, but that guy is unable to figure that one out. Looking at the implementation, it looks pretty ad hoc.

Jan 27 2017, 8:25 AM
andreadb accepted D29097: [X86][SSE] Lower scalar_to_vector(0) to zero vector.

Thanks Simon,
Looks good to me.

Jan 27 2017, 3:51 AM

Jan 26 2017

andreadb added a comment to D29088: Do not create ctlz/cttz(X, false) when the target do not support zero defined ctlz/cttz..

Can you clarify why the flag is suboptimal by itself? The intrinsic carries the same semantic as the unfolded sequence, isn't it?

Jan 26 2017, 9:11 AM
andreadb added a comment to D29088: Do not create ctlz/cttz(X, false) when the target do not support zero defined ctlz/cttz..

Thanks Amaury,

Jan 26 2017, 5:45 AM

Jan 25 2017

andreadb updated subscribers of D29088: Do not create ctlz/cttz(X, false) when the target do not support zero defined ctlz/cttz..
Jan 25 2017, 3:13 AM
andreadb added a comment to D29088: Do not create ctlz/cttz(X, false) when the target do not support zero defined ctlz/cttz..

@andreadb it hides the branch many of the subsequent optimization passes, resulting in bad codegen. I stumbled on bad codegen from cttz/ctlz several time recently. Thing that I noticed are : doing the 0 case check several time, failure to constant fold the 0 case when there is one, etc...

Jan 25 2017, 3:10 AM

Jan 24 2017

andreadb added a comment to D29088: Do not create ctlz/cttz(X, false) when the target do not support zero defined ctlz/cttz..

The branch/select is removed in InstCombine and then recreated in CodegenPrepare, effectively making it invisible to most of the optimization pipeline, which lead to bad optimizations of cttz/ctlz when the target doesn't have a zero defined version of them.

Jan 24 2017, 10:23 AM
andreadb updated subscribers of D29081: Use InstCombine's builder in foldSelectCttzCtlz instead of creating a new one..
Jan 24 2017, 6:38 AM

Jan 5 2017

andreadb accepted D27811: [CostModel][X86] Add support for broadcast shuffle costs.

LGTM. Thanks Simon!

Jan 5 2017, 7:20 AM
andreadb added inline comments to D27811: [CostModel][X86] Add support for broadcast shuffle costs.
Jan 5 2017, 3:53 AM

Jan 4 2017

andreadb added inline comments to D27811: [CostModel][X86] Add support for broadcast shuffle costs.
Jan 4 2017, 10:59 AM

Dec 15 2016

andreadb added a comment to D27804: [SimplifyCFG] Merge debug locations when hoisting an instruction from a then/else branch. NFC..

Thanks for the review Adrian.

Dec 15 2016, 10:19 AM
andreadb added a comment to D27811: [CostModel][X86] Add support for broadcast shuffle costs.

I noticed that getShuffleCost is becoming quite big.

You're not kidding ;-)

Do you think it makes sense to split the logic in getShuffleCost in three parts (a function for each supported ShuffleKind)?. In case, getShuffleCost could be refactored in a separate commit.

I have considered hijacking the 'ISD::VECTOR_SHUFFLE' int entry in both CostTbleEmtry and CostTableLookup to do lookup based on TTI::ShuffleKind instead - what do you think?

Dec 15 2016, 10:05 AM
andreadb added a comment to D27811: [CostModel][X86] Add support for broadcast shuffle costs.

Hi Simon,

Dec 15 2016, 8:24 AM
andreadb accepted D27684: [X86][SSE] Fix domains for VZEXT_LOAD type instructions.

LGTM.

Dec 15 2016, 6:55 AM
andreadb accepted D27692: [x86] use a single shufps when it can save instructions.

OK to commit this and D27684?

Dec 15 2016, 6:32 AM
andreadb retitled D27804: [SimplifyCFG] Merge debug locations when hoisting an instruction from a then/else branch. NFC. from to [SimplifyCFG] Merge debug locations when hoisting an instruction from a then/else branch. NFC..
Dec 15 2016, 6:30 AM

Dec 14 2016

andreadb added inline comments to D27714: [DAGCombiner] Use SelectionDAG::isKnownToBeAPowerOfTwo instead of just APInt::isPowerOf2.
Dec 14 2016, 6:41 AM
andreadb added a comment to D27692: [x86] use a single shufps when it can save instructions.

I'd like to propose the following:

1 - we get this patch and D27684 approved and committed, providing v4i32 lowering to shufps and avoiding some of the more unnecessary domain switches.
2 - get shufps lowering added to target shuffle combining, I added shufpd recently and it's just been the domain issues that I wanted to tidyup up before adding shufps as well
3 - add support for v8i32 (and v16i32?) lowering to shufps
4 - other missing domain switch patterns (scalar stores and vpermilps/vpshufd come to mind)
5 - add support for domain switching to target shuffle combine when the shuffle depth is 3 or more - this will allow pshufd use on pre-AVX targets and seems to introduce some good uses of insertps as well.

That seems within scope for 4.0 and doesn't involve anything too exotic. After 4.0 we should be in a better position to begin work on moving some of this work to MC combines to better make use of specific scheduler models

Dec 14 2016, 4:41 AM
andreadb accepted D27714: [DAGCombiner] Use SelectionDAG::isKnownToBeAPowerOfTwo instead of just APInt::isPowerOf2.

Hi Simon,

Dec 14 2016, 4:38 AM

Dec 8 2016

andreadb added inline comments to D27573: [X86] Fix bug in r288804..
Dec 8 2016, 11:29 AM

Dec 6 2016

andreadb added a comment to D27468: When GVN removes a redundant load, it should not modify the debug location of the dominating load..

This makes sense to me. Are there other cases where we should invoke the mergeDebugLoc API that we've been discussing in D26256?

Dec 6 2016, 1:06 PM
andreadb added a comment to D27462: For functions with debug info, do not propagate the callsite debug location to inlined instructions..

Hi David,

Dec 6 2016, 11:56 AM
andreadb retitled D27468: When GVN removes a redundant load, it should not modify the debug location of the dominating load. from to When GVN removes a redundant load, it should not modify the debug location of the dominating load..
Dec 6 2016, 9:46 AM
andreadb added inline comments to D27461: [DAGCombine] Add (sext_in_reg (zext x)) -> (sext x) combine.
Dec 6 2016, 9:33 AM
andreadb accepted D27461: [DAGCombine] Add (sext_in_reg (zext x)) -> (sext x) combine.

Hi Simon,
I only have one question. Otherwise the patch LGMT.

Dec 6 2016, 9:02 AM
andreadb retitled D27462: For functions with debug info, do not propagate the callsite debug location to inlined instructions. from to For functions with debug info, do not propagate the callsite debug location to inlined instructions..
Dec 6 2016, 8:37 AM

Nov 22 2016

andreadb accepted D26985: [X86] Simplify lowerVectorShuffleAsBitMask to handle only integer VT's.

Sounds reasonable to me.
In practice, lowerVectorShuffleAsBitMask is only called when the shuffle valuetype is integer. In all other cases, the lowering logic forces a bitcast on the shuffle operands.

Nov 22 2016, 12:17 PM
andreadb added a comment to D26678: [X86] Remove dead code from LowerVectorBroadcast.

LGTM. Thanks Zvi!

Nov 22 2016, 7:08 AM
andreadb accepted D26678: [X86] Remove dead code from LowerVectorBroadcast.
Nov 22 2016, 6:44 AM
andreadb added inline comments to D26678: [X86] Remove dead code from LowerVectorBroadcast.
Nov 22 2016, 4:23 AM

Nov 17 2016

andreadb added inline comments to D26790: [X86] Add a hasOneUse check to selectScalarSSELoad to keep the same load from being folded multiple times.
Nov 17 2016, 4:00 AM

Nov 9 2016

andreadb added inline comments to D26335: [ms] Reinstate https://reviews.llvm.org/D14748 after https://reviews.llvm.org/D20291.
Nov 9 2016, 3:27 AM

Oct 25 2016

andreadb retitled D25953: [IndVarSimplify][DebugLoc] When widening the exit loop condition, correctly reuse the debug location of the original comparison. from to [IndVarSimplify][DebugLoc] When widening the exit loop condition, correctly reuse the debug location of the original comparison..
Oct 25 2016, 11:17 AM
andreadb added a comment to D25872: [IndVarSimplify][DebugLoc] When widening the IV increment, correctly set the debug loc..

Seems fine to me.

Out of curiosity: Does the SCE debugger make use of DWARF discriminators? Or is this used for profiling?

Oct 25 2016, 9:25 AM

Oct 22 2016

andreadb accepted D25524: [DAGCombine] Preserve shuffles when one of the vector operands is constant.
Oct 22 2016, 3:13 AM
andreadb added a comment to D25524: [DAGCombine] Preserve shuffles when one of the vector operands is constant.

Thanks Zvi!

Oct 22 2016, 1:40 AM

Oct 21 2016

andreadb added a comment to D25580: [PowerPC] Improve handling of BUILD_VECTOR nodes (integer results) - Abandoned.

Hi Nemanja,

<snip>

The other thing I have noticed is that no extra tests have been added by your patch. Given the amount of extra logic added by the patch, I have to admit that I was a bit surprised not to see any extra test coverage. Is it because the coverage for those features is already in the regression suite?

I will certainly re-work this to try to split it up into a few patches. However, I'm not sure why you say there is no new test coverage. The test case "build-vector-tests.ll" is part of the patch and it is a completely new test case. I have a feeling that Phabricator has collapsed it in your view and you've just missed it.

Oct 21 2016, 10:41 AM
andreadb added a comment to D25872: [IndVarSimplify][DebugLoc] When widening the IV increment, correctly set the debug loc..

Hi David,

Oct 21 2016, 9:24 AM
andreadb added a comment to D25580: [PowerPC] Improve handling of BUILD_VECTOR nodes (integer results) - Abandoned.

Hi Nemanja,

Oct 21 2016, 7:36 AM
andreadb retitled D25872: [IndVarSimplify][DebugLoc] When widening the IV increment, correctly set the debug loc. from to [IndVarSimplify][DebugLoc] When widening the IV increment, correctly set the debug loc..
Oct 21 2016, 7:34 AM

Oct 19 2016

andreadb added inline comments to D25524: [DAGCombine] Preserve shuffles when one of the vector operands is constant.
Oct 19 2016, 4:49 AM