[llvm-commits] [PATCH] Add support for promoting integer SETCC

[Jakob Stoklund Olesen]

On 24/07/2009, at 18.43, Eli Friedman wrote:

> You didn't actually change it precisely per my suggestion... you
> should be using isOperationLegal instead of isOperationLegalOrCustom.

Sorry, I wrongly assumed that was a typo =:-O

If SETCC is marked "Custom", won't it be OK to create such a node at  
this point?
Won't the custom LowerOperation() be called after SimplifySetCC() in  
that case?
Note that PowerPC and X86 have "Custom" SETCC operations, so the patch  
could be changing their behaviour.

Clearly, the CondCode check should match the SETCC check:

either isOperationLegal() && getCondCodeAction() == Legal (as you wrote)
or isOperationLegalOrCustom() && isCondCodeLegal() (as in the patch)

The last method name looks like a bug in this context. It should  
probably have been isCondCodeLegalOrCustom() considering the  
implementation.

>> Considering the recent discussion about commit policy, is this type  
>> of
>> bugfix patch "trivial" enough to review-after-commit?
>
> Yes, this type of change is small enough to review-after-commit, if
> you're confident the change is correct.

Thanks,

/jakob