Edwin was asking about how we should handle PR3328, how we should make GEP respect -fwrapv etc. I wrote up some thoughts here if anyone is interested: http://nondot.org/sabre/LLVMNotes/IntegerOverflow.txt -Chris
Sounds ambitious! A comprehensive, efficient trapv would be excellent. gcc's implementation seems quite incomplete, for example it fails to trap overflows in the constant folder. John Regehr
On 07 Feb 2009, at 23:17, Chris Lattner wrote:> Edwin was asking about how we should handle PR3328, how we should make > GEP respect -fwrapv etc. I wrote up some thoughts here if anyone is > interested: > http://nondot.org/sabre/LLVMNotes/IntegerOverflow.txtThe proposal suggests to change/split the existing sub/add/mul opcodes. This makes me wonder to what extent it is (currently, or ever) advisable for an external compiler to generate LLVM IR. Is there a plan to stabilise at some point and guarantee backwards compatibility to a certain extent, or should compilers that are not integrated in the LLVM infrastructure always target one particular release of LLVM? Jonas
On 2009-02-08, at 05:59, Jonas Maebe wrote:> The proposal suggests to change/split the existing sub/add/mul > opcodes. This makes me wonder to what extent it is (currently, or > ever) advisable for an external compiler to generate LLVM IR. Is > there a plan to stabilise at some point and guarantee backwards > compatibility to a certain extent, or should compilers that are not > integrated in the LLVM infrastructure always target one particular > release of LLVM?LLVM does guarantee backwards compatibility with compiled bitcode. The C++ interfaces are not frozen, so you may need to upgrade code targeting LLVM when upgrading; reasonable efforts are made to avoid making this process painful. Of course, what code is contributed to the project will be maintained through these changes. — Gordon
Hi Chris,
Would it be better to split add into multiple opcodes instead of using
SubclassData bits? Compare this:
switch (I->getOpcode()) {
case Instruction::Add: {
switch (cast<Add>(I)->getOverflowBehavior()) {
case AddInstruction::Wrapping:
// ...
case AddInstruction::UndefinedSigned:
// ...
case AddInstruction::UndefinedUnsigned:
// ...
}
}
}
with this:
switch (I->getOpcode()) {
case Instruction::Add:
// ...
case Instruction::SAdd_Open:
// ...
case Instruction::UAdd_Open:
// ...
break;
}
I'm not sure about the name "Open"; fixed-size integers are
"closed"
under
wrapping and saturating add, so "open" sort of suggests an
alternative,
and is concise. But regardless, a one-level switch seems more
convenient
than a two-level one.
It's a little less convenient in the case of code that wants to handle
all the
flavors of add the same way, but it still seems worth it.
Encoding might be a concern, as Sub, Mul, Div, and Rem would all have
variants, but there are plenty of bits in SubclassID, and it doesn't
look like
the bitcode representation uses packed opcode fields.
Dan
On Feb 7, 2009, at 2:17 PM, Chris Lattner wrote:
> Edwin was asking about how we should handle PR3328, how we should make
> GEP respect -fwrapv etc. I wrote up some thoughts here if anyone is
> interested:
> http://nondot.org/sabre/LLVMNotes/IntegerOverflow.txt
>
> -Chris
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
On Feb 7, 2009, at 7:53 PM, John Regehr wrote:> Sounds ambitious! A comprehensive, efficient trapv would be > excellent. > gcc's implementation seems quite incomplete, for example it fails to > trap > overflows in the constant folder.GCC's implementation has a huge number of problems, and I really don't think that implementing trapv in llvm-gcc would fare much better (fold mangles trees severely). Clang preserves and hands full unmangled source-level ASTs to codegen, so codegen could handle this properly. That said, I don't know of anyone interested in implementing this in the short term. -Chris
On Feb 8, 2009, at 8:58 AM, Dan Gohman wrote:> Hi Chris, > > Would it be better to split add into multiple opcodes instead of using > SubclassData bits?No, I don't think so. The big difference here is that (like type) "opcode" never changes for an instruction once it is created. I expect that optimizations would want to play with these (e.g. convert them to 'undefined' when it can prove overflow never happens) so I think it is nice to not have it be in the opcode field. This also interacts with FP rounding mode stuff, which I expect to handle the same way with FP operations some day.> Compare this: > > switch (I->getOpcode()) { > case Instruction::Add: { > switch (cast<Add>(I)->getOverflowBehavior()) { > case AddInstruction::Wrapping: > // ... > case AddInstruction::UndefinedSigned: > // ... > case AddInstruction::UndefinedUnsigned:Sure, that is ugly. However, I think it would be much more common to look at these in "isa" flavored tests than switches: if (isa<SAdd_OpenInst>(X)) is much nicer than: if (BinaryOperator *BO = dyn_cast<BinaryOperator>(x)) if (BO->getOpcode() == blah::Add && BO->getOverflow() == blah::Undefined) However, we a) already suffer this just for Add, because we don't have an AddInst class, and b) don't care about the opcode anyway. IntrinsicInst is a good example of how we don't actually need opcode bits or concrete classes to make isa "work". It would be a nice cleanup to add new "pseudo instruction" classes like IntrinsicInst for all the arithmetic anyway. If the switch case really does become important, we can just add a getOpcodeWithSubtypes() method that returns a new flattened enum.>-Chris