[llvm-commits] x86 branch sequence optimization in LLVM code gen: please review

Thu Jan 5 00:11:51 PST 2012

Evan, 

I think that PTEST should be optimized by teaching "X86TargetLowering::computeMaskedBitsForTargetNode" that PTEST only sets the lowest bit, and not using a peephole. 

We will also need to teach "SimplifySetCC" to use "SimplifyDemandedBits".

Nadav

-----Original Message-----
From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Evan Cheng
Sent: Thursday, January 05, 2012 09:48
To: Umansky, Victor
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] x86 branch sequence optimization in LLVM code gen: please review

Minor nit-pick:

+// Helper which returns index of constant operand of a two-operand node.
+static inline int GetConstOpIndexFor2OpNode(SDValue Op) {
+  if (dyn_cast<ConstantSDNode>(Op.getOperand(0)))
+    return 0;
+  else if (dyn_cast<ConstantSDNode>(Op.getOperand(1)))
+    return 1;
+  else 
+    return -1;
+}

Please replace dyn_cast<ConstantSDNode> with isa<ConstantSDNode>. Please also remove the last "else". That's the preferred llvm style.

Thanks,

Evan

On Jan 4, 2012, at 11:31 PM, Umansky, Victor wrote:

> A correction:
> 
> Elena Demikhovsky (elena.demikhovsky at intel.com) reviewed the updated patch (face-to-face, in Intel).
> 
> Victor
> 
> -----Original Message-----
> From: Umansky, Victor
> Sent: Thursday, January 05, 2012 09:25
> To: 'Evan Cheng'
> Cc: Bruno Cardoso Lopes; llvm-commits at cs.uiuc.edu
> Subject: RE: [llvm-commits] x86 branch sequence optimization in LLVM 
> code gen: please review
> 
> No, I haven't received any feedback.
> Would you?
> 
> Victor
> 
> -----Original Message-----
> From: Evan Cheng [mailto:evan.cheng at apple.com]
> Sent: Thursday, January 05, 2012 09:09
> To: Umansky, Victor
> Cc: Bruno Cardoso Lopes; llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] x86 branch sequence optimization in LLVM 
> code gen: please review
> 
> Has someone reviewed the updated patch?
> 
> Evan
> 
> On Dec 21, 2011, at 8:10 AM, Umansky, Victor wrote:
> 
>> Replacing patch with a newer one. 
>> Changes: aligned naming policy with that of LLVM.
>> 
>> Victor
>> 
>> -----Original Message-----
>> From: llvm-commits-bounces at cs.uiuc.edu 
>> [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Umansky, 
>> Victor
>> Sent: Wednesday, December 21, 2011 11:01
>> To: Bruno Cardoso Lopes
>> Cc: llvm-commits at cs.uiuc.edu
>> Subject: Re: [llvm-commits] x86 branch sequence optimization in LLVM 
>> code gen: please review
>> 
>> Hi Bruno,
>> 
>> I've updated the patch with additional peephole-optimization patterns and with additional AVX-specific LIT test.
>> 
>> Can you please review, so that I'll proceed to commit the fixes?
>> 
>> Best Regards,
>>  Victor
>> 
>> -----Original Message-----
>> From: llvm-commits-bounces at cs.uiuc.edu 
>> [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Umansky, 
>> Victor
>> Sent: Thursday, December 15, 2011 20:09
>> To: Bruno Cardoso Lopes
>> Cc: llvm-commits at cs.uiuc.edu
>> Subject: Re: [llvm-commits] x86 branch sequence optimization in LLVM 
>> code gen: please review
>> 
>> Hi Bruno,
>> 
>> Please see attached the patch.
>> 
>> It incorporates the feedback, and I yet extended it with coverage for more LLVM IR patterns related to usage of ptestz/ptestc LLVM built-ins (tests are also extended).
>> 
>> Best Regards,
>>  Victor
>> 
>> 
>> -----Original Message-----
>> From: Bruno Cardoso Lopes [mailto:bruno.cardoso at gmail.com]
>> Sent: Friday, December 09, 2011 04:25
>> To: Umansky, Victor
>> Cc: Chad Rosier; Anton Korobeynikov; bruno.cardoso at gmail.com; 
>> llvm-commits at cs.uiuc.edu
>> Subject: Re: [llvm-commits] x86 branch sequence optimization in LLVM 
>> code gen: please review
>> 
>> On Wed, Dec 7, 2011 at 1:03 PM, Umansky, Victor <victor.umansky at intel.com> wrote:
>>> 
>>> Hi Chad, Anton, Bruno,
>>> 
>>> 
>>> 
>>> Thank you for the suggestion.
>>> 
>>> 
>>> 
>>> Unfortunately, it won't work in the case of brcond.ll file.
>>> 
>>> 
>>> 
>>> Indeed I can introduce different "check-prefix" values in order to separate checks for "core2" case from those for "penryn" case.
>>> 
>>> However, the compilation of all functions in a file will be done unconditionally for both "RUN" cases. And this will inevitably lead to the test failure (in instruction selection) when a function using "ptest" LLVM intrinsic will be processed with "-mcpu=core2" option.
>>> 
>>> That's why I was not able to include the test cases for "ptest" intrinsic sequence to a file which will be compiled for a pre-Penryn target.
>>> 
>>> 
>>> 
>>> A solution which does work is to have legacy brcond.ll LIT tests running under "-mcpu=penryn".
>>> 
>>> I'm attaching the file.
>>> 
>>> Are you OK with such solution?
>> 
>> LGTM, please resend the orignal patch with the testcase (both in the 
>> same patch file)! Also remove the trailing CRs and generate the diff 
>> with "svn diff" under the project root. If you have any question, the 
>> docs may help: http://llvm.org/docs/DeveloperPolicy.html#patches
>> 
>> --
>> Bruno Cardoso Lopes
>> http://www.brunocardoso.cc
>> ---------------------------------------------------------------------
>> 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.
>> ---------------------------------------------------------------------
>> 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.
>> <brcond_combine.patch>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> ---------------------------------------------------------------------
> 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.
> 

_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
---------------------------------------------------------------------
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.