llvm dev - Nov 2012 - [LLVMdev] [RFC] Extend LLVM IR to express "fast-math" at a per-instruction level

On Nov 14, 2012, at 12:47 PM, Chris Lattner <clattner at apple.com> wrote:
> 
> On Nov 14, 2012, at 12:28 PM, Michael Ilseman <milseman at apple.com>
wrote:
> 
>> I think I missed what problem we're trying to solve here.
>> 
>> I'm looking at implementing the bitcode now. I have code to
successfully read and write out the LLVM IR textual formal (LLParser, etc) and
set the corresponding SubclassOptionalData bits. Looking at LLVMBitCodes.h,
I'm seeing where these bits reside in the bitcode, so I believe that things
should be pretty straight-forward from here.
>> 
>> Joe, what are the reasons for me to increment the IR version number? My
understanding is that I'll just be using existing bits that were previously
ignored. Ignoring these bits is still valid, just conservative. I believe these
flags would be zero-ed out in old IR (correct me if I'm wrong), which is the
intended default.
> 
> Yes, this is the right thing, just make the sense of the bits in the
bitcode file be "1" for cases that differs from the old default.
> 
Will do!
>> Chris, what problem could be solved by adding extra operands to binary
ops? I'm trying to avoid those sorts of modifications, as the fast-math
flags could make sense applied to a variety operations, e.g. comparisons and
casts.
> 
> How, specifically, are you proposing that these bits be encoded?
> 
I'm new to the bitcode so let me know if this doesn't make sense. I was
going to look at the encoding for nuw (OBO_NO_UNSIGNED_WRAP) and follow how it
is encoded/decoded in the bitcode. I would then specify some kind of fast-math
enum and encode it in a similar fashion.

After I go down this path a little more I might be able to give you a more
intelligent answer.

Thanks!
> -Chris
>

I attached a working patch of changes to the bitcode reader and writer. This
patch references other local changes I have to other parts of the code (e.g.
"FastMathFlags"), but shows the general idea I'm going for. When
I've ironed out all the bugs, I'll attach a series of patches for all
the other content.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: bitcode_example.patch
Type: application/octet-stream
Size: 2928 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20121114/12a06609/attachment.obj>
-------------- next part --------------


Does this patch make sense, or am I still missing the main concern?

On Nov 14, 2012, at 1:39 PM, Michael Ilseman <milseman at apple.com>
wrote:
> 
> On Nov 14, 2012, at 12:47 PM, Chris Lattner <clattner at apple.com>
wrote:
> 
>> 
>> On Nov 14, 2012, at 12:28 PM, Michael Ilseman <milseman at
apple.com> wrote:
>> 
>>> I think I missed what problem we're trying to solve here.
>>> 
>>> I'm looking at implementing the bitcode now. I have code to
successfully read and write out the LLVM IR textual formal (LLParser, etc) and
set the corresponding SubclassOptionalData bits. Looking at LLVMBitCodes.h,
I'm seeing where these bits reside in the bitcode, so I believe that things
should be pretty straight-forward from here.
>>> 
>>> Joe, what are the reasons for me to increment the IR version
number? My understanding is that I'll just be using existing bits that were
previously ignored. Ignoring these bits is still valid, just conservative. I
believe these flags would be zero-ed out in old IR (correct me if I'm
wrong), which is the intended default.
>> 
>> Yes, this is the right thing, just make the sense of the bits in the
bitcode file be "1" for cases that differs from the old default.
>> 
> 
> Will do!
> 
>>> Chris, what problem could be solved by adding extra operands to
binary ops? I'm trying to avoid those sorts of modifications, as the
fast-math flags could make sense applied to a variety operations, e.g.
comparisons and casts.
>> 
>> How, specifically, are you proposing that these bits be encoded?
>> 
> 
> I'm new to the bitcode so let me know if this doesn't make sense. I
was going to look at the encoding for nuw (OBO_NO_UNSIGNED_WRAP) and follow how
it is encoded/decoded in the bitcode. I would then specify some kind of
fast-math enum and encode it in a similar fashion.
> 
> After I go down this path a little more I might be able to give you a more
intelligent answer.
> 
> Thanks!
> 
>> -Chris
>> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

On Nov 14, 2012, at 5:13 PM, Michael Ilseman <milseman at apple.com>
wrote:
> I attached a working patch of changes to the bitcode reader and writer.
This patch references other local changes I have to other parts of the code
(e.g. "FastMathFlags"), but shows the general idea I'm going for.
When I've ironed out all the bugs, I'll attach a series of patches for
all the other content.
> 
> <bitcode_example.patch>
> 
> Does this patch make sense, or am I still missing the main concern?
> 
Michael,

Ah I see.  So this just becomes a matter of interpretation of bits in the
optimization flags.  Shouldn't need to promote the CurVersion.

Nitpick: 80-cols in BitcodeReader.cpp

Since Instruction::FastMathFlags is a class, seems like the constructor could
take in Record[OpNum] , and assign the flags.

Looking forward to the full patch.

Joe

On Nov 14, 2012, at 1:39 PM, Michael Ilseman <milseman at apple.com>
wrote:
> 
>>> Chris, what problem could be solved by adding extra operands to
binary ops? I'm trying to avoid those sorts of modifications, as the
fast-math flags could make sense applied to a variety operations, e.g.
comparisons and casts.
>> 
>> How, specifically, are you proposing that these bits be encoded?
>> 
> 
> I'm new to the bitcode so let me know if this doesn't make sense. I
was going to look at the encoding for nuw (OBO_NO_UNSIGNED_WRAP) and follow how
it is encoded/decoded in the bitcode. I would then specify some kind of
fast-math enum and encode it in a similar fashion.
Right, this is what I was suggesting. NSW and friends are handle with this code:

      assert(isa<BinaryOperator>(I) && "Unknown
instruction!");
      Code = bitc::FUNC_CODE_INST_BINOP;
      if (!PushValueAndType(I.getOperand(0), InstID, Vals, VE))
        AbbrevToUse = FUNCTION_INST_BINOP_ABBREV;
      pushValue(I.getOperand(1), InstID, Vals, VE);
      Vals.push_back(GetEncodedBinaryOpcode(I.getOpcode()));
      uint64_t Flags = GetOptimizationFlags(&I);
      if (Flags != 0) {
        ...
        Vals.push_back(Flags);
      }

Basically, the Flags operand is added to the end of the operand list (before the
bitcode record is written) if present.  That's what I meant as an
"extra operand", but I see how that was probably *really* confusing :)

-Chris

On Nov 14, 2012, at 5:23 PM, Michael Ilseman <milseman at apple.com>
wrote:
>> Ah I see.  So this just becomes a matter of interpretation of bits in
the optimization flags.  Shouldn't need to promote the CurVersion.
>> 
>> Nitpick: 80-cols in BitcodeReader.cpp
>> 
>> Since Instruction::FastMathFlags is a class, seems like the constructor
could take in Record[OpNum] , and assign the flags.
>> 
> 
> I like the intent, but unfortunately Record[OpNum] is just a uint64_t. The
agreement of which bit means what is in LLVMBitCodes.h, and I'd prefer not
having an implicit handshake between the bitcode and the rest of LLVM. However,
I'll try to find ways to factor more convenience into shared code.
It's also a current design policy to explicitly enumerate the bitcode code
separately from internal codes.  This is because there should be no binary
compatibility concerns with renumbering internal IR enums, but if bitcode uses
them directly, we'd have a problem.

-Chris