[polly] block generators: fix insertion of values in BBMap

Tobias Grosser tobias at grosser.es
Fri May 3 06:03:00 PDT 2013


On 05/02/2013 11:19 PM, Sebastian Pop wrote:
> Hi,
>
> When BBMap contains a new value for an old value, i.e., BBMap.count(Old) > 0,
> BBMap[Old] should not be NULL.  The attached patch adds an assert to verify this
> property.

That is OK.

> The patch also transforms the insertions into BBMap to avoid storing NULL
> pointers in the map.  I do not know why the existing code stores NULL pointers:
>
>    BBMap[Load] = generateScalarLoad(...);
>
> whereas storing the value into a temporary before inserting in BBMap does not:
>
>    Value *NewLoad = generateScalarLoad(...);
>    BBMap[Load] = NewLoad;
>
> Looks like a bug due to the order of evaluation of the arguments of a function.

This is very surprising. I don't really understand how this fixes the 
problem. Can you provide a reduced test case?

> Ok to commit?

I would first like to understand the problem. I do not yet see the 
reason why the transformation you propose should avoid the generation of 
NULL pointers. I also don't see where those pointers would come from in 
the first place.

Can you point out what exactly is the bug caused by the order of 
evaluation of arguments?

Cheers,
Tobias




More information about the llvm-commits mailing list