[PATCH] "float2int": Add a new pass to demote from float to int where possible.

[James Molloy]

Hi Hal,

Thanks for the quick review! Sorry for not getting back to it for a couple of days - I was at a conference.

I've uploaded a new patch addressing most of your comments.

> You can also handle Select here (and PHIs, although that might require more work?)


Yes, I think I can. I've added a FIXME for this and will do (at least the select work) in a followup, if that's OK? PHIs I think will certainly need more thought.

> Why is the FIXME here? Do you want to mark these as unsigned under some circumstances?


The FIXME was (a) not meant to make it to upstream code review :) and (b) because I hadn't clocked the sense of APSFloat's constructor - I thought it was "isSigned" rather than "isUnsigned", and my current code felt like it was doing the wrong thing. It's actually right.

> Why are you checking for isFullSet() here? What about if you only have i8 (or something definitively smaller than the mantissa)?


R's underlying type is i65 (MaxBitwidth + 1), so isFullSet() will only trigger on either an i64 multiplication/addition or a poison value (which I define as "i65 full-set"). 0..255 should never trigger isFullSet here, unless I'm misunderstanding how ConstantRange works.

> Hrmm, we have other floating-point types in the IR. You need to either exclude them somewhere, or handle them.


Thanks! I wasn't aware of the fltSemantics precision. Updated.

> Exactly where does 23 come from here?


Thanks for noticing that. It's wrong, it should of course be 32. I don't really want to use getIntNTy() because I don't want to get an illegal type - I've restricted myself to i32 or i64 for the moment. I could grab TLI and query that I suppose?

> Hrmm. To have converted an instruction, you must have converted all uses, right?


In fact, because I'm using a MapVector and reversing through it I hit all nodes after their uses. So I just don't need the use_empty() check.


REPOSITORY
  rL LLVM

http://reviews.llvm.org/D7790

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/