[llvm-commits] [PATCH] Teach IRBuilder about simplifying BinOp(Value, Constant)

[Török Edwin]

On 2009-01-04 21:35, Dan Gohman wrote:
> On Jan 4, 2009, at 2:55 AM, Török Edwin wrote:
>
>   
>> On 2009-01-04 05:26, Dan Gohman wrote:
>>     
>>> I fixed utils/vim/vimrc; it should now do the right thing.
>>>
>>>       
>> Thanks.
>> Since you know more about vim indenting than me, here are a couple  
>> more
>> things that I have to do manually, to make my code look like
>> the rest in IRBuilder.h (I assume these are part of the LLVM coding
>> style too?):
>> - class is not indented vs namespace (vim wants to indent)
>> - public: is not indented vs class (vim wants to indent)
>> - when I wrap arguments due to exceeding 80-col, vim doesn't align the
>> beginning with the ( on previous line, so I have to manually tab/space
>> align it
>>
>> Are these fixable?
>>     
>
> I did the public: and argument ones.  I didn't see an easy way to do
> the namespace one.
>   

Thanks, that helps a lot!

>   
>>>             (P <= *I && P > UI->getParent()->begin()))
>>>
>>> BasicBlock::iterator is not random-access so it doesn't support <=  
>>> or >.
>>> Unfortunately, what's probably happening here is both sides being
>>> implicitly converted to pointers which are then compared, which is
>>> not good.  This is the dark side of LLVM's style of iterators :-(.  I
>>> just
>>> added some code which I believe will catch this error at compile-time
>>> now.
>>>
>>>       
>> Oops. In that case I only check that the instruction is in the same
>> BasicBlock, and require the caller
>> to not move the insertion point backwards. This should ensure that the
>> Value I return exists at the current insertionpoint.
>> [I could iterate and check that the instruction is really there, but
>> that would be a waste of time, since I know it is...]
>> Since this is a default off feature (requiring an explicit template
>> parameter to activate) I think it is ok.
>> What do you think?
>>     
>
> How about having it act as if reuseOps is false when InsertPt is not
> equal to BB->end()?  Would that still be permissive enough for your
> needs?

Yes, that is OK. I added a private function getReuse() that will either
return a BB (if reuseOps is true, and InsertPt == BB->end()), or 0
otherwise.


Best regards,
--Edwin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: simplifier5.patch
Type: text/x-diff
Size: 27834 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20090104/94164600/attachment.patch>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: interdiff.txt
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20090104/94164600/attachment.txt>