llvm.org GIT mirror
Merging r296003: ------------------------------------------------------------------------ r296003 | mcrosier | 2017-02-23 10:49:03 -0800 (Thu, 23 Feb 2017) | 32 lines [Reassociate] Add negated value of negative constant to the Duplicates list. In OptimizeAdd, we scan the operand list to see if there are any common factors between operands that can be factored out to reduce the number of multiplies (e.g., 'A*A+A*B*C+D' -> 'A*(A+B*C)+D'). For each operand of the operand list, we only consider unique factors (which is tracked by the Duplicate set). Now if we find a factor that is a negative constant, we add the negated value as a factor as well, because we can percolate the negate out. However, we mistakenly don't add this negated constant to the Duplicates set. Consider the expression A*2*-2 + B. Obviously, nothing to factor. For the added value A*2*-2 we over count 2 as a factor without this change, which causes the assert reported in PR30256. The problem is that this code is assuming that all the multiply operands of the add are already reassociated. This change avoids the issue by making OptimizeAdd tolerate multiplies which haven't been completely optimized; this sort of works, but we're doing wasted work: we'll end up revisiting the add later anyway. Another possible approach would be to enforce RPO iteration order more strongly. If we have RedoInsts, we process them immediately in RPO order, rather than waiting until we've finished processing the whole function. Intuitively, it seems like the natural approach: reassociation works on expression trees, so the optimization only works in one direction. That said, I'm not sure how practical that is given the current Reassociate; the "optimal" form for an expression depends on its use list (see all the uses of "user_back()"), so Reassociate is really an iterative optimization of sorts, so any changes here would probably get messy. PR30256 Differential Revision: https://reviews.llvm.org/D30228 ------------------------------------------------------------------------ git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_40@296156 91177308-0d34-0410-b5e6-96231b3b80d8 Hans Wennborg 2 years ago
2 changed file(s) with 24 addition(s) and 4 deletion(s).
 1520 1520 if (ConstantInt *CI = dyn_cast(Factor)) { 1521 1521 if (CI->isNegative() && !CI->isMinValue(true)) { 1522 1522 Factor = ConstantInt::get(CI->getContext(), -CI->getValue()); 1523 assert(!Duplicates.count(Factor) && 1524 "Shouldn't have two constant factors, missed a canonicalize");⏎ 1523 if (!Duplicates.insert(Factor).second)⏎ 1524 continue; 1525 1525 unsigned Occ = ++FactorOccurrences[Factor]; 1526 1526 if (Occ > MaxOcc) { 1527 1527 MaxOcc = Occ; 1533 1533 APFloat F(CF->getValueAPF()); 1534 1534 F.changeSign(); 1535 1535 Factor = ConstantFP::get(CF->getContext(), F); 1536 assert(!Duplicates.count(Factor) && 1537 "Shouldn't have two constant factors, missed a canonicalize");⏎ 1536 if (!Duplicates.insert(Factor).second)⏎ 1537 continue; 1538 1538 unsigned Occ = ++FactorOccurrences[Factor]; 1539 1539 if (Occ > MaxOcc) { 1540 1540 MaxOcc = Occ;
 221 221 ; CHECK-LABEL: @test15 222 222 ; CHECK: and i1 %A, %B 223 223 } 224 225 ; PR30256 - previously this asserted. 226 ; CHECK-LABEL: @test16 227 ; CHECK: %[[FACTOR:.*]] = mul i64 %a, -4 228 ; CHECK-NEXT: %[[RES:.*]] = add i64 %[[FACTOR]], %b 229 ; CHECK-NEXT: ret i64 %[[RES]] 230 define i64 @test16(i1 %cmp, i64 %a, i64 %b) { 231 entry: 232 %shl = shl i64 %a, 1 233 %shl.neg = sub i64 0, %shl 234 br i1 %cmp, label %if.then, label %if.end 235 236 if.then: ; preds = %entry 237 %add1 = add i64 %shl.neg, %shl.neg 238 %add2 = add i64 %add1, %b 239 ret i64 %add2 240 241 if.end: ; preds = %entry 242 ret i64 0 243 }