[llvm-commits] [Patch] Use TargetTransformInfo to control switch-to-lookup table transform

[Hal Finkel]

----- Original Message -----
> From: "Duncan Sands" <baldrick at free.fr>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Nadav Rotem" <nadav.rotem at intel.com>, llvm-commits at cs.uiuc.edu
> Sent: Monday, October 29, 2012 11:50:12 AM
> Subject: Re: [llvm-commits] [Patch] Use TargetTransformInfo to control switch-to-lookup table transform
> 
> Hi Hal,
> 
>  >>> When the switch-to-lookup tables transform landed in
>  >>> SimplifyCFG,
> >>> it
> >>> was pointed out [1] that this could be inappropriate for some
> >>> targets.
> >>> Since there was no way at the time for the pass to know anything
> >>> about
> >>> the target, an awkward reverse-transform was added in
> >>> CodeGenPrepare
> >>> that turned lookup tables back into switches for some targets.
> >>>
> >>> The attached patch uses the new TargetTransformInfo to determine
> >>> if
> >>> a
> >>> switch should be transformed, and removes
> >>> CodeGenPrepare::ConvertLoadToSwitch. Please take a look!
> >>
> >> personally I think it is completely wrong to have codegen concepts
> >> such as
> >> ISD::BR_JT and ISD::BRIND leaking into the IR level:
> >
> > But these concepts are not leaking into the IR layer. These are
> > contained in Target (in lib/Target/TargetTransformImpl.cpp
> 
> nope, Hans added it to Local.h, not to TargetTransformImpl.cpp.

I think that your favorite patch viewer is playing tricks on you. I just checked again, and those things are in:

diff --git a/lib/Target/TargetTransformImpl.cpp b/lib/Target/TargetTransformImpl.cpp
index d3823a6..f21e2f1 100644
--- a/lib/Target/TargetTransformImpl.cpp
+++ b/lib/Target/TargetTransformImpl.cpp
@@ -49,6 +49,12 @@ unsigned ScalarTargetTransformImpl::getJumpBufSize() const {
   return TLI->getJumpBufSize();
 }
 
+bool ScalarTargetTransformImpl::shouldBuildLookupTables() const {
+  return TLI->supportJumpTables() &&
+      (TLI->isOperationLegalOrCustom(ISD::BR_JT, MVT::Other) ||
+       TLI->isOperationLegalOrCustom(ISD::BRIND, MVT::Other));
+}
+

The changes to local are:
diff --git a/include/llvm/Transforms/Utils/Local.h b/include/llvm/Transforms/Utils/Local.h
index 49eeb57..5c804d8 100644
--- a/include/llvm/Transforms/Utils/Local.h
+++ b/include/llvm/Transforms/Utils/Local.h
@@ -37,6 +37,7 @@ class AllocaInst;
 class ConstantExpr;
 class DataLayout;
 class TargetLibraryInfo;
+class TargetTransformInfo;
 class DIBuilder;
 
 template<typename T> class SmallVectorImpl;
@@ -134,7 +135,8 @@ bool EliminateDuplicatePHINodes(BasicBlock *BB);
 /// of the CFG.  It returns true if a modification was made, possibly deleting
 /// the basic block that was pointed to.
 ///
-bool SimplifyCFG(BasicBlock *BB, const DataLayout *TD = 0);
+bool SimplifyCFG(BasicBlock *BB, const DataLayout *TD = 0,
+                 const TargetTransformInfo *TTI = 0);

 -Hal

> 
> Ciao, Duncan.
> 
> ), and the IR layer does not see them. Also, a target could override
> this
> implementation and provide a different one. Nevertheless, the IR
> layer only sees
> an abstract interface, and if Target is not linked in, just gets a
> fixed generic
> answer.
> >
> >   -Hal
> >
> >>
> >>> --- a/include/llvm/Transforms/Utils/Local.h
> >>> +++ b/include/llvm/Transforms/Utils/Local.h
> >>> @@ -49,6 +49,12 @@ unsigned
> >>> ScalarTargetTransformImpl::getJumpBufSize() const {
> >>>     return TLI->getJumpBufSize();
> >>>   }
> >>>
> >>> +bool ScalarTargetTransformImpl::shouldBuildLookupTables() const
> >>> {
> >>> +  return TLI->supportJumpTables() &&
> >>> +      (TLI->isOperationLegalOrCustom(ISD::BR_JT, MVT::Other) ||
> >>> +       TLI->isOperationLegalOrCustom(ISD::BRIND, MVT::Other));
> >>> +}
> >>> +
> >>>   //===----------------------------------------------------------------------===//
> >>>   //
> >>>   // Calls used by the vectorizers.
> >>
> >> Ciao, Duncan.
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>
> >
> 
> 

-- 
Hal Finkel
Postdoctoral Appointee
Leadership Computing Facility
Argonne National Laboratory