Refactor CallDescriptor::Flags handling

These flags are later packed into an InstructionCode by the
instruction selector, but not all bits are preserved. This CL adds a
few assertions and helper methods to clarify and make this a bit
safer.

Bug: v8:8888
Change-Id: I01adc9f7887cc21c623d790dc204ac1d4b9bc7ac
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2388116
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69728}

src/compiler/backend/code-generator-impl.h

@@ -259,6 +259,12 @@ class OutOfLineCode : public ZoneObject {
 
 inline bool HasCallDescriptorFlag(Instruction* instr,
                                   CallDescriptor::Flag flag) {
+  STATIC_ASSERT(CallDescriptor::kFlagsBitsEncodedInInstructionCode == 10);
+#ifdef DEBUG
+  static constexpr int kInstructionCodeFlagsMask =
+      ((1 << CallDescriptor::kFlagsBitsEncodedInInstructionCode) - 1);
+  DCHECK_EQ(static_cast<int>(flag) & kInstructionCodeFlagsMask, flag);
+#endif
   return MiscField::decode(instr->opcode()) & flag;
 }
 

src/compiler/backend/code-generator.cc

@@ -973,15 +973,13 @@ Label* CodeGenerator::AddJumpTable(Label** targets, size_t target_count) {
 }
 
 void CodeGenerator::RecordCallPosition(Instruction* instr) {
-  CallDescriptor::Flags flags(MiscField::decode(instr->opcode()));
-
-  bool needs_frame_state = (flags & CallDescriptor::kNeedsFrameState);
-
+  const bool needs_frame_state =
+      HasCallDescriptorFlag(instr, CallDescriptor::kNeedsFrameState);
   RecordSafepoint(instr->reference_map(), needs_frame_state
                                               ? Safepoint::kLazyDeopt
                                               : Safepoint::kNoLazyDeopt);
 
-  if (flags & CallDescriptor::kHasExceptionHandler) {
+  if (HasCallDescriptorFlag(instr, CallDescriptor::kHasExceptionHandler)) {
     InstructionOperandConverter i(this, instr);
     RpoNumber handler_rpo = i.InputRpo(instr->InputCount() - 1);
     DCHECK(instructions()->InstructionBlockAt(handler_rpo)->IsHandler());

src/compiler/backend/instruction-selector.cc

@@ -2737,10 +2737,20 @@ void InstructionSelector::VisitParameter(Node* node) {
 }
 
 namespace {
+
 LinkageLocation ExceptionLocation() {
   return LinkageLocation::ForRegister(kReturnRegister0.code(),
                                       MachineType::IntPtr());
 }
+
+constexpr InstructionCode EncodeCallDescriptorFlags(
+    InstructionCode opcode, CallDescriptor::Flags flags) {
+  // Note: Not all bits of `flags` are preserved.
+  STATIC_ASSERT(CallDescriptor::kFlagsBitsEncodedInInstructionCode ==
+                MiscField::kSize);
+  return opcode | MiscField::encode(flags & MiscField::kMax);
+}
+
 }  // namespace
 
 void InstructionSelector::VisitIfException(Node* node) {
@@ -2863,6 +2873,7 @@ void InstructionSelector::VisitCall(Node* node, BasicBlock* handler) {
 #if ABI_USES_FUNCTION_DESCRIPTORS
       // Highest misc_field bit is used on AIX to indicate if a CFunction call
       // has function descriptor or not.
+      STATIC_ASSERT(MiscField::kSize == kHasFunctionDescriptorBitShift + 1);
       if (!call_descriptor->NoFunctionDescriptor()) {
         misc_field |= 1 << kHasFunctionDescriptorBitShift;
       }
@@ -2871,18 +2882,18 @@ void InstructionSelector::VisitCall(Node* node, BasicBlock* handler) {
       break;
     }
     case CallDescriptor::kCallCodeObject:
-      opcode = kArchCallCodeObject | MiscField::encode(flags);
+      opcode = EncodeCallDescriptorFlags(kArchCallCodeObject, flags);
       break;
     case CallDescriptor::kCallJSFunction:
-      opcode = kArchCallJSFunction | MiscField::encode(flags);
+      opcode = EncodeCallDescriptorFlags(kArchCallJSFunction, flags);
       break;
     case CallDescriptor::kCallWasmCapiFunction:
     case CallDescriptor::kCallWasmFunction:
     case CallDescriptor::kCallWasmImportWrapper:
-      opcode = kArchCallWasmFunction | MiscField::encode(flags);
+      opcode = EncodeCallDescriptorFlags(kArchCallWasmFunction, flags);
       break;
     case CallDescriptor::kCallBuiltinPointer:
-      opcode = kArchCallBuiltinPointer | MiscField::encode(flags);
+      opcode = EncodeCallDescriptorFlags(kArchCallBuiltinPointer, flags);
       break;
   }
 
@@ -2960,7 +2971,7 @@ void InstructionSelector::VisitTailCall(Node* node) {
         return;
     }
   }
-  opcode |= MiscField::encode(call_descriptor->flags());
+  opcode = EncodeCallDescriptorFlags(opcode, call_descriptor->flags());
 
   Emit(kArchPrepareTailCall, g.NoOutput());
 

src/compiler/linkage.h

@@ -205,6 +205,9 @@ class V8_EXPORT_PRIVATE CallDescriptor final
     kCallBuiltinPointer,     // target is a builtin pointer
   };
 
+  // NOTE: The lowest 10 bits of the Flags field are encoded in InstructionCode
+  // (for use in the code generator). All higher bits are lost.
+  static constexpr int kFlagsBitsEncodedInInstructionCode = 10;
   enum Flag {
     kNoFlags = 0u,
     kNeedsFrameState = 1u << 0,
@@ -226,6 +229,10 @@ class V8_EXPORT_PRIVATE CallDescriptor final
     // AIX has a function descriptor by default but it can be disabled for a
     // certain CFunction call (only used for Kind::kCallAddress).
     kNoFunctionDescriptor = 1u << 9,
+
+    // Flags past here are *not* encoded in InstructionCode and are thus not
+    // accessible from the code generator. See also
+    // kFlagsBitsEncodedInInstructionCode.
   };
   using Flags = base::Flags<Flag>;