[PATCH] D25200: [InstCombine] New opportunities for FoldAndOfICmp and FoldXorOfICmp

Ehsan Amiri via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 24 15:07:05 PDT 2016

amehsan added inline comments.

Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:744-750
+// For an ICMP where RHS is zero, we want to check if the ICMP is equivalent to
+// comparing a group of bits in an integer value against zero. If yes we return
+// (true, X, Mask). X is a boolean to identify condition code. Mask identifies
+// BitGroup. This is mostly when LHS is 'and' of an integer with a mask, but
+// there are other cases as well. Like when CC=SLT, where effectively we check
+// to see if sign bit is one or not.
+static std::tuple<bool, bool, const APInt *>
amehsan wrote:
> majnemer wrote:
> > A tuple with multiple bool types can be a little confusing. I'd prefer a little struct. At the very least, this needs to be documented.
> I figured that I will need to add some comments even if I have a struct, because the field names will not be clear enough. So I concluded creating a struct will not be a significant help to clarify things and so I added explanation to the comments before the function to explain each component of the return value. Will update the patch shortly.
Actually, I changed my mind about this. Will add an struct and update the patch.


More information about the llvm-commits mailing list