[llvm-commits] [llvm] r165126 - /llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp

[Nick Lewycky]

Preston Gurd wrote:
> Author: pgurd
> Date: Wed Oct  3 11:11:44 2012
> New Revision: 165126
>
> URL: http://llvm.org/viewvc/llvm-project?rev=165126&view=rev
> Log:
> This Patch corrects a problem whereby the optimization to use a faster divide
> instruction (for Intel Atom) was not being done by Clang, because
> the type context used by Clang is not the default context.

No, this is wrong. Please revert this patch.

The point of having multiple contexts is to permit two users of 
llvm-as-a-library in a single program to work without interfering with 
each other. Even just converting an integer type from context A to a 
different context would cause a data race between this and another 
thread that's using the global context.

There's a special exception for the ilist end iterator using 
getGlobalContext(), but pretty much anything else that isn't the 
top-level program calling getGlobalContext() is a bug.

I think the bug is in lib/Target/X86/X86ISelLowering.cpp where we add 
the slow bypass types using getGlobalContext(). And I have bad news, 
which is that you don't have the context at the point where your code 
that neesd it currently is (and indeed, none of the rest of the 
constructor relies on having it either). Is there a reason you need 
Type* in BypassSlowDivTypes, or could you just have the bitwidth of 
integers? (ie., do you care about vectors?)

Nick

> It fixes the problem by getting the global context types for each div/rem
> instruction in order to compare them against the types in the BypassTypeMap.
>
> Tests for this will be done as a separate patch to Clang.
>
> Patch by Tyler Nowicki.
>
>
> Modified:
>      llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp
>
> Modified: llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp?rev=165126&r1=165125&r2=165126&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp Wed Oct  3 11:11:44 2012
> @@ -238,14 +238,24 @@
>       if (!UseDivOp&&  !UseRemOp)
>         continue;
>
> +    // Skip division on vector types, only optimize integer instructions
> +    if (!J->getType()->isIntegerTy())
> +      continue;
> +
> +    // Get same type in global context
> +    IntegerType *T = cast<IntegerType>(J->getType());
> +    IntegerType *GT = IntegerType::get(getGlobalContext(), T->getBitWidth());
> +
>       // Continue if div/rem type is not bypassed
> -    DenseMap<Type *, Type *>::const_iterator BT =
> -      BypassTypeMap.find(J->getType());
> -    if (BT == BypassTypeMap.end())
> +    DenseMap<Type *, Type *>::const_iterator BI = BypassTypeMap.find(GT);
> +    if (BI == BypassTypeMap.end())
>         continue;
>
> -    IntegerType *BypassType = cast<IntegerType>(BT->second);
> -    MadeChange |= reuseOrInsertFastDiv(F, I, J, BypassType, UseDivOp,
> +    // Get the bypass type in the original context
> +    IntegerType *GBT = cast<IntegerType>(BI->second);
> +    IntegerType *BT = IntegerType::get(J->getContext(), GBT->getBitWidth());
> +
> +    MadeChange |= reuseOrInsertFastDiv(F, I, J, BT, UseDivOp,
>                                          UseSignedOp, DivCache);
>     }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>