[llvm-commits] [PATCH] Bug fix to enable zextload on targets that support it

[Silviu Baranga]

Hi Alon,

 

Don't we need isConstantSplat to get the lane masks in the first place?

The isConstantSplat function also does other things like handling

undefined operands.

 

Of course, if you would find a way of optimizing that code while preserving 

the functionality, that would be great.

 

Sorry for the late reply.

 

Cheers,

Silviu

 

From: Mishne, Alon [mailto:alon.mishne at intel.com] 
Sent: 03 September 2012 12:23
To: Silviu Baranga; llvm-commits at cs.uiuc.edu
Cc: James Molloy
Subject: RE: [llvm-commits] [PATCH] Bug fix to enable zextload on targets
that support it

 

Hi Silviu,

 

Thanks for letting me know, I didn't understand the loop is actually
required - and you are of course right.

By the way, since the loop is required anyway, I don't really see the
benefit of using isConstantSplat() over just anding the lanes (paying in
isConstantSplat() time to just decrease AND iterations). But it's a small
thing, of course :)

 

Thanks for correcting me! My patch should not be applied.

-          Alon

 

From: Silviu Baranga [mailto:silbar01 at arm.com] 
Sent: Tuesday, August 28, 2012 11:55
To: Mishne, Alon; llvm-commits at cs.uiuc.edu
Cc: James Molloy
Subject: RE: [llvm-commits] [PATCH] Bug fix to enable zextload on targets
that support it

 

Hi Alon,

 

I noticed that you are removing the code that ands every lane

of the splat value. That code(the loop) is actually required, since

we might not have the same value for all lanes and we would

still want to fold the AND node.

 

Related to this, I've sent a patch  a couple of days ago to handle

the same issue:

http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120820/148839.
html

 

Cheers,

Silviu

 

 

From: llvm-commits-bounces at cs.uiuc.edu
[mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Mishne, Alon
Sent: 26 August 2012 08:27
To: llvm-commits at cs.uiuc.edu
Subject: [llvm-commits] [PATCH] Bug fix to enable zextload on targets that
support it

 

Hi,

 

I've noticed a clever trick in lib/CodeGen/SelectionDAG/DAGCombiner.cpp of
identifying an AND whose only purpose is to reset the top bits as a
potential zextload instead of extload in targets that support it. However,
the code for identifying the constant used in such an AND with a vector is
wrong - it makes incorrect usage of the isConstantSplat() method, assuming
it works differently from how it really does.

 

Attached is a patch that fixes this by using that method correctly, enabling
a nice optimization on targets that support zextload. The fixed code is also
much simpler and more efficient than the incorrect usage, becomes it removes
an unneeded loop.

 

Please review this patch and commit if acceptable,

-          Alon

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120905/c2a4bdc7/attachment.html>