[gimple] PHI-opt bug, advice needed
Richard, I got the min/max VRP optimization I mentioned working, but it uncovered a bug in phiopts where that pass is constructing MIN and MAX exprs. (Funnily enough, I only found that failure due to a broken test of decimal fp). We start with the following bit of code: // X has unknown range here if (X < 0) X = 0; if (X > 191) X = 191; // X known to be [0,191] here What's happening is that after mergephi3, (just before phiopt) we have 2: if X_6 < 0 goto 5 3: if X_6 > 191 goto 5 6: 5: X_2 = PHI<0(2), 191(3), X_6(6)> X_2 has range info marking it to be [0,191] because of an earlier VRP pass. This is all good, but notice the PHI on X_2 has 3 incoming edges. phiopt first converts BB3, BB6 into a MIN_EXPR which is correct, but it clones X_2 to hold that result. That cloning duplicates X_2's range information, which is not correct for the result of the MIN_EXPR. (this incorrect info causes my new optimization to elide a MAX_EXPR that is actually needed). Normally it would be ok, because the PHI on X_2 would usually only have 2 incoming edges (from the conditional and the fall-through). The code in tree-ssa-phiopt.c (minmax_replacement) is: result = duplicate_ssa_name (PHI_RESULT (phi), NULL); new_stmt = gimple_build_assign (result, minmax, arg0, arg1); gsi = gsi_last_bb (cond_bb); gsi_insert_before (&gsi, new_stmt, GSI_NEW_STMT); replace_phi_edge_with_variable (cond_bb, e1, phi, result); Is the right solution to (a) use make_ssa_name instead of duplicate_ssa_name, (iff PHI has more than 2 edges?)? Or is it for (b) replace_phi_edge_with_variable to zap RESULT's range info if the PHI isn't collapse completely? Or is it something else? More generally, when is it appropriate to use make_ssa_name and when to use duplicate_ssa_name? nathan
Re: [gimple] PHI-opt bug, advice needed
On August 10, 2015 11:49:15 AM GMT+02:00, Nathan Sidwell wrote: >Richard, >I got the min/max VRP optimization I mentioned working, but it >uncovered a bug >in phiopts where that pass is constructing MIN and MAX exprs. (Funnily >enough, >I only found that failure due to a broken test of decimal fp). > >We start with the following bit of code: > >// X has unknown range here > if (X < 0) > X = 0; > if (X > 191) > X = 191; >// X known to be [0,191] here > >What's happening is that after mergephi3, (just before phiopt) we have >2: if X_6 < 0 goto 5 >3: if X_6 > 191 goto 5 >6: >5: X_2 = PHI<0(2), 191(3), X_6(6)> > >X_2 has range info marking it to be [0,191] because of an earlier VRP >pass. This >is all good, but notice the PHI on X_2 has 3 incoming edges. > >phiopt first converts BB3, BB6 into a MIN_EXPR which is >correct, but >it clones X_2 to hold that result. That cloning duplicates X_2's range > >information, which is not correct for the result of the MIN_EXPR. (this > >incorrect info causes my new optimization to elide a MAX_EXPR that is >actually >needed). > >Normally it would be ok, because the PHI on X_2 would usually only have >2 >incoming edges (from the conditional and the fall-through). > >The code in tree-ssa-phiopt.c (minmax_replacement) is: > result = duplicate_ssa_name (PHI_RESULT (phi), NULL); > new_stmt = gimple_build_assign (result, minmax, arg0, arg1); > gsi = gsi_last_bb (cond_bb); > gsi_insert_before (&gsi, new_stmt, GSI_NEW_STMT); > > replace_phi_edge_with_variable (cond_bb, e1, phi, result); > >Is the right solution to (a) use make_ssa_name instead of >duplicate_ssa_name, >(iff PHI has more than 2 edges?)? Or is it for (b) >replace_phi_edge_with_variable to zap RESULT's range info if the PHI >isn't >collapse completely? Or is it something else? If stmts are effectively put under different controlling predicates then their defs have to have flow sensitive info pruned. I added a helper function for that recently (don't remember its name...). For the case in question using make SSA name would also work. > >More generally, when is it appropriate to use make_ssa_name and when to >use >duplicate_ssa_name? Duplicate duplicates all on-the-side info which is OK if the value of the new name is the same as that of the old one. You might need to prube the flow sensitive parts if its def is placed in a different location. Richard. >nathan
Re: CFI directives and dynamic stack alignment
On Mon, Aug 03, 2015 at 02:48:09PM -0700, Steve Ellcey wrote: > When I generate code to dynamically align the stack my code looks like > this: > > fn2: > .frame $fp,32,$31 # vars= 0, regs= 2/0, args= 16, gp= 8 > .mask 0xc000,-4 > .fmask 0x,0 > .setnoreorder > .setnomacro > lui $2,%hi(null) > li $3,-16 # 0xfff0 > lw $2,%lo(null)($2) > and $sp,$sp,$3 > addiu $sp,$sp,-32 > .cfi_def_cfa_offset 32 > sw $fp,24($sp) > .cfi_offset 30, -8 > move$fp,$sp > .cfi_def_cfa_register 30 > sw $31,28($sp) > .cfi_offset 31, -4 > jal abort > sb $0,0($2) > > The 'and' instruction is where the stack gets aligned and if I remove that > one instruction, everything works. I think I need to put out some new CFI > psuedo-ops to handle this but I am not sure what they should be. I am just > not very familiar with the CFI directives. I don't speak mips assembly very well, but it looks to me that you have more than just CFI problems. How do you restore sp on return from the function, assuming sp wasn't 16-byte aligned to begin with? Past that "and $sp,$sp,$3" you don't have any means of calculating the original value of sp! (Which of course is why you also can't find a way of representing the frame address.) -- Alan Modra Australia Development Lab, IBM
