[llvm] r337065 - [X86][SLH] Regroup the instructions in isDataInvariantLoad a little. NFC

Fri Jul 13 15:41:46 PDT 2018

Author: ctopper
Date: Fri Jul 13 15:41:46 2018
New Revision: 337065

URL: http://llvm.org/viewvc/llvm-project?rev=337065&view=rev
Log:
[X86][SLH] Regroup the instructions in isDataInvariantLoad a little. NFC

-Move BSF/BSR to the same group as TZCNT/LZCNT/POPCNT.
-Split some of the bit manipulation instructions away from TZCNT/LZCNT/POPCNT. These are things like 'x & (x - 1)' which are composed of a few simple arithmetic operations. These aren't nearly as complicated/surprising as counting bits.
-Move BEXTR/BZHI into their own group. They aren't like a simple arithmethic op or the bit manipulation instructions. They're more like a shift+and.

Differential Revision: https://reviews.llvm.org/D49312

Modified:
    llvm/trunk/lib/Target/X86/X86SpeculativeLoadHardening.cpp

Modified: llvm/trunk/lib/Target/X86/X86SpeculativeLoadHardening.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86SpeculativeLoadHardening.cpp?rev=337065&r1=337064&r2=337065&view=diff
==============================================================================

--- llvm/trunk/lib/Target/X86/X86SpeculativeLoadHardening.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86SpeculativeLoadHardening.cpp Fri Jul 13 15:41:46 2018
@@ -800,9 +800,9 @@ static bool isDataInvariantLoad(MachineI
     // By default, assume that the load will immediately leak.
     return false;
 
-    // On x86 it is believed that imul is constant time w.r.t. the loaded data.
-    // However, they set flags and are perhaps the most surprisingly constant
-    // time operations so we call them out here separately.
+  // On x86 it is believed that imul is constant time w.r.t. the loaded data.
+  // However, they set flags and are perhaps the most surprisingly constant
+  // time operations so we call them out here separately.
   case X86::IMUL16rm:
   case X86::IMUL16rmi8:
   case X86::IMUL16rmi:
@@ -813,10 +813,29 @@ static bool isDataInvariantLoad(MachineI
   case X86::IMUL64rmi32:
   case X86::IMUL64rmi8:
 
-    // Bitfield and bit scanning instructions that are somewhat surprisingly
-    // constant time as they scan across bits and do other fairly complex
-    // operations like popcnt, but are believed to be constant time on x86.
-    // However, these set flags.
+  // Bit scanning and counting instructions that are somewhat surprisingly
+  // constant time as they scan across bits and do other fairly complex
+  // operations like popcnt, but are believed to be constant time on x86.
+  // However, these set flags.
+  case X86::BSF16rm:
+  case X86::BSF32rm:
+  case X86::BSF64rm:
+  case X86::BSR16rm:
+  case X86::BSR32rm:
+  case X86::BSR64rm:
+  case X86::LZCNT16rm:
+  case X86::LZCNT32rm:
+  case X86::LZCNT64rm:
+  case X86::POPCNT16rm:
+  case X86::POPCNT32rm:
+  case X86::POPCNT64rm:
+  case X86::TZCNT16rm:
+  case X86::TZCNT32rm:
+  case X86::TZCNT64rm:
+
+  // Bit manipulation instructions are effectively combinations of basic
+  // arithmetic ops, and should still execute in constant time. These also
+  // set flags.
   case X86::BLCFILL32rm:
   case X86::BLCFILL64rm:
   case X86::BLCI32rm:
@@ -837,21 +856,19 @@ static bool isDataInvariantLoad(MachineI
   case X86::BLSMSK64rm:
   case X86::BLSR32rm:
   case X86::BLSR64rm:
-  case X86::BZHI32rm:
-  case X86::BZHI64rm:
-  case X86::LZCNT16rm:
-  case X86::LZCNT32rm:
-  case X86::LZCNT64rm:
-  case X86::POPCNT16rm:
-  case X86::POPCNT32rm:
-  case X86::POPCNT64rm:
-  case X86::TZCNT16rm:
-  case X86::TZCNT32rm:
-  case X86::TZCNT64rm:
   case X86::TZMSK32rm:
   case X86::TZMSK64rm:
 
-    // Basic arithmetic is constant time on the input but does set flags.
+  // Bit extracting and clearing instructions should execute in constant time,
+  // and set flags.
+  case X86::BEXTR32rm:
+  case X86::BEXTR64rm:
+  case X86::BEXTRI32mi:
+  case X86::BEXTRI64mi:
+  case X86::BZHI32rm:
+  case X86::BZHI64rm:
+
+  // Basic arithmetic is constant time on the input but does set flags.
   case X86::ADC8rm:
   case X86::ADC16rm:
   case X86::ADC32rm:
@@ -870,12 +887,6 @@ static bool isDataInvariantLoad(MachineI
   case X86::AND64rm:
   case X86::ANDN32rm:
   case X86::ANDN64rm:
-  case X86::BSF16rm:
-  case X86::BSF32rm:
-  case X86::BSF64rm:
-  case X86::BSR16rm:
-  case X86::BSR32rm:
-  case X86::BSR64rm:
   case X86::OR8rm:
   case X86::OR16rm:
   case X86::OR32rm:
@@ -892,10 +903,6 @@ static bool isDataInvariantLoad(MachineI
   case X86::XOR16rm:
   case X86::XOR32rm:
   case X86::XOR64rm:
-  case X86::BEXTR32rm:
-  case X86::BEXTR64rm:
-  case X86::BEXTRI32mi:
-  case X86::BEXTRI64mi:
     // Check whether the EFLAGS implicit-def is dead. We assume that this will
     // always find the implicit-def because this code should only be reached
     // for instructions that do in fact implicitly def this.
@@ -910,13 +917,13 @@ static bool isDataInvariantLoad(MachineI
     // don't set EFLAGS.
     LLVM_FALLTHROUGH;
 
-    // Integer multiply w/o affecting flags is still believed to be constant
-    // time on x86. Called out separately as this is among the most surprising
-    // instructions to exhibit that behavior.
+  // Integer multiply w/o affecting flags is still believed to be constant
+  // time on x86. Called out separately as this is among the most surprising
+  // instructions to exhibit that behavior.
   case X86::MULX32rm:
   case X86::MULX64rm:
 
-    // Arithmetic instructions that are both constant time and don't set flags.
+  // Arithmetic instructions that are both constant time and don't set flags.
   case X86::PDEP32rm:
   case X86::PDEP64rm:
   case X86::PEXT32rm:
@@ -930,8 +937,8 @@ static bool isDataInvariantLoad(MachineI
   case X86::SHRX32rm:
   case X86::SHRX64rm:
 
-    // Conversions are believed to be constant time and don't set flags.
-    // FIXME: Add AVX versions.
+  // Conversions are believed to be constant time and don't set flags.
+  // FIXME: Add AVX versions.
   case X86::CVTSD2SI64rm_Int:
   case X86::CVTSD2SIrm_Int:
   case X86::CVTSS2SI64rm_Int:
@@ -945,7 +952,7 @@ static bool isDataInvariantLoad(MachineI
   case X86::CVTTSS2SIrm:
   case X86::CVTTSS2SIrm_Int:
 
-    // Loads to register don't set flags.
+  // Loads to register don't set flags.
   case X86::MOV8rm:
   case X86::MOV8rm_NOREX:
   case X86::MOV16rm:


