[llvm-commits] [Patch] Make GlobalOpt conservative with TLS vars (PR14309, 3.2-blocker)

[Hans Wennborg]

On Mon, Nov 12, 2012 at 7:31 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Mon, Nov 12, 2012 at 8:20 AM, Hans Wennborg <hans at chromium.org> wrote:
>> Hi all,
>>
>> The attached patch takes a stab at fixing PR14309 which was marked as
>> a 3.2-blocker.
>>
>> For global variables which get the same value stored into them
>> everywhere, GlobalOpt will replace them with a constant. The problem
>> is that a thread-local GlobalVariable looks like one value, but is
>> different between threads.
>>
>> My patch introduces Constant::isThreadDependent() which returns true
>> for thread-local variables and constants which depend on them (e.g. a
>> GEP into a thread-local array), and teaches GlobalOpt not to track
>> such values.
>>
>> Please take a look!
>>
>> Also, if anyone thinks there are more passes which assume that
>> thread-local GlobalVariables have the same value everywhere, please
>> let me know.
>
>
> +/// isThreadDependent - Return true if the value can vary between threads.
> +bool Constant::isThreadDependent() const {
> +  if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(this))
> +    return GV->isThreadLocal();
> +
> +  for (unsigned I = 0, E = getNumOperands(); I != E; ++I) {
> +    if (const Constant *C = dyn_cast<Constant>(getOperand(I))) {
> +      if (C->isThreadDependent())
> +        return true;
> +    }
> +  }
> +
> +  return false;
> +}
>
> Dumb recursion is exponential time over an arbitrary ConstantExpr.

Attaching new patch that makes sure to only check each unique Constant* once.

Thanks,
Hans
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr14309_tls_globalopt2.diff
Type: application/octet-stream
Size: 4673 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121113/17ad81ff/attachment.obj>