[PATCH] Added InstCombine transformation for combining two instructions icmp ult/ule/uge/ugt (ashr/lshr (Const2) %A), (Const1)

[Tilmann Scheller]

Hi Ankur,

can you please add a comment which indicates the whole transformation you
are doing?

You only seem to mention what you are matching on: (icmp ugt/uge/ult/ule
(ashr/lshr const2, A), const1) but not what you are transforming into.
Looking at the tests makes it obvious which transformation is being done but
if someone just sees the code this is significantly harder to deduce.

In fact I think you could add a couple more comments to FoldUICmp...() to
make it more clear what's going on, especially with all the different
branches.

Thanks for working on this!

Regards,

Tilmann

-----Original Message-----
From: llvm-commits-bounces at cs.uiuc.edu
[mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Ankur Garg
Sent: Friday, November 14, 2014 5:41 AM
To: ankur29.garg at samsung.com; dexonsmith at apple.com;
Andrea_DiBiagio at sn.scee.net; suyog.sarda at samsung.com;
david.majnemer at gmail.com
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] Added InstCombine transformation for combining two
instructions icmp ult/ule/uge/ugt (ashr/lshr (Const2) %A), (Const1)

Gentle Ping !

http://reviews.llvm.org/D5518



_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits