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

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 5:04 PM, Joe Abbey <jabbey at arxan.com> wrote:
> 
> 
> 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.
> 
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.
> Looking forward to the full patch.
> 
> Joe

Thanks for the feedback!

>> 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.
> 

Works for me.

Joe