[PATCH] D59547: [X86] Remove X86 specific dag nodes for RDTSC/RDTSCP/RDPMC. NFCI

Tue Mar 19 13:59:35 PDT 2019

craig.topper added inline comments.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:23090
     SmallVector<SDValue, 2> Results;
-    getReadPerformanceCounter(Op.getNode(), dl, DAG, Subtarget, Results);
+    // Handles the lowering of builtin intrinsics that read performance monitor
+    // counters (x86_rdpmc).
----------------
andreadb wrote:
> andreadb wrote:
> > craig.topper wrote:
> > > I can't decide if these should really be in the X86IntrinsicsInfo.h table. We're not using the Opc0 field. So we're treating them as one offs. Which makes me wonder if they shouldn't just be in the switch that is used when the table lookup fails.
> > > 
> > > Or maybe we should merge RDPMC and XGETBV using the Opc0 field?
> > I think it is a good idea to just merge these two cases and simply write:
> > ```
> > expandIntrinsicWChainHelper(Op.getNode(), dl, DAG, IntrData->Opc0, X86::ECX,
> >                                 Subtarget, Results);
> > ```
> Actually, we could also merge `case RDTSC` along RDPMC and XGETBV .
> In the case of RDTSC, function `getReadTimeStampCounter()` simply delegates to `expandIntrinsicWChainHelper()`.
> 
> Would it be okay if for now I merge these three cases togheter?
Sounds good.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59547/new/

https://reviews.llvm.org/D59547