[PATCH] D49238: [InstCombine] add more SPFofSPF folding

ChenZheng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 12 08:15:42 PDT 2018


shchenz added a comment.

Some explanation for this patch:
1, for TODO in InstCombineSelect.cpp: because matchSelectPattern() can recognize more abs pattern, including abs(-x), then the assert in canonicalizeAbsNabs() will be hit. But we decide to canonicalize pattern abs(-x) in a separated patch, I just keep canonicalizeAbsNabs() recognize the patterns which trunk recognizes. When canonicalization patch is committed, we should solve TODO.

2, Reply the comment in https://reviews.llvm.org/D48754 in file ValueTracking.cpp at line 4671. I think matchselectpattern can only return a SelectPatternResult struct. RHS stores negated operand, LHS stores the other operand.  ABS(-X) and NABS(-x) is just for good understanding?

3, testcase abs_abs_x17/abs_nabs_x17/nabs_abs_x17/nabs_nabs_x17 already be folded in trunk is because   (%sub = sub nsw i32 0, %x
, %cmp = icmp sgt i32 %sub, -1) can be folded to (%sub = sub nsw i32 0, %x, %cmp = icmp slt i32 %x, 0) by visiticmp(). After this folding, the left IRs is a standard abs pattern(abx(x)), so it can be recognized. But after this patch, abs(-x) is also recognized, so visiticmp() can not fold %cmp = icmp sgt i32 %sub, -1 any more. So we get different result with trunk. But after  canonicalize patch, I assume, we should also get abs(x)


https://reviews.llvm.org/D49238





More information about the llvm-commits mailing list