[LLDB] Add basic floating point ops to IR interpreter

[kpdev42]

Summary

Patch adds support for fadd, fsub, fdiv, fmul and fcmp to IR interpreter.

~~~

OS Laboratory. Huawei RRI. Saint-Petersburg

[DavidSpickett]

I know very little about the interpreter so someone else can comment there.

Certainly seems like a useful change. Added some generic comments.

lldb/test/API/lang/c/fpeval/TestFPEval.py

2

Update this comment

22

I'm pretty sure anything starting with test will do, so I'd just call this def test(self):.

23

Update this comment

52

Probably a silly question but do you need != here? I guess it becomes the inverse of == but wouldn't cost much to test it.

lldb/test/API/lang/c/fpeval/main.c

4

I don't think you need any of these includes, certainly the latter 2.

10

Small thing but could this line just be return 0; just saves a few cycles where you might think this line is being tested somehow when it really isn't.

[kpdev42]

Address review notes

[clayborg]

Will the floating point emulation be perfect compared to actually JITing the code? If we are going to enable this feature I would like to see tests that actually JIT code do the same operations somehow to verify that things match up with the IR interpreted results produce. The main reason we didn't include floating point code in the IR interpreter before is it is harder to make things match up with exactly how things are going to be run if it were JIT'ed into the process. On x86_64 or x64 there are floating point settings that can be changed and affect how operations happen in the current thread. The main example I always think of is how older debuggers would create recursive descent parsers and evaluate floating point operations using native floating point types. I believe that our APFloat classes can do everything correctly, but I would like to see tests that verify that we get the same results as if we were running actual code in the process. For this you can add an extra function to the lldb/test/API/lang/c/fpeval/main.c source like:

typedef enum FloatOperation { add, subtract, multiply, ... };
double eval(double a, double b, FloatOperation op) {
  switch (op) {
  case add: return a+b;
  case subtract: return a-b;
  ...
  }
}

lldb/test/API/lang/c/fpeval/TestFPEval.py

33

You might be able to get away with not actually even creating a target or running to main if you define variables in your expression:

self.expect('expr --allow-jit false  -- float x=2.2; float y=4.4; x+y', ...

lldb/test/API/lang/c/fpeval/main.c

2–11

I don't think this file is even needed if you define variables locally in your own expression each time like I mentioned above ("expr float x=2.2; float y=4.4; x+y")

4–5

We should be testing "float" and "long double" as well to verify those work.

[clayborg]

lldb/test/API/lang/c/fpeval/TestFPEval.py

33–34

if we want to verify if the above "a+b" works as expected compared to JITed code, you can also run an expression like:

expr eval(a, b, add)

Then then we would want to compare the expression results to make sure the binary answer matches exactly. To do this, we will want to use the LLDB native APIs:

no_jit_options = lldb.SBExpressionOptions()
no_jit_options = expt_options.SetAllowJIT(False)
jit_options = lldb.SBExpressionOptions()
jit_options = expt_options.SetAllowJIT(True)
no_jit_value = frame.EvaluateExpression("a+b", no_jit_options)
jit_value = frame.EvaluateExpression("eval(a, b, add)", jit_options)
no_jit_data = no_jit_value.GetData()
jit_data = no_jit_value.GetData()

Then we need to compare the data byte for byte.

[kpdev42]

lldb/test/API/lang/c/fpeval/TestFPEval.py

33

Actually I wanted the test to read floating point values from process memory, checking if they're passed correctly to IR along the way. This might be redundant, however your expression still needs a target to be executed.

33–34

IMO this will not work, because lldb always tries interpreter first and jitter is used only if interpreter fails. This may be circumvented by generating expression which interpreter cannot handle (function call?), however I don't see a point in doing it. Why not just check result value?

lldb/test/API/lang/c/fpeval/main.c

4–5

Long double is not supported by this patch, but can be added. However long double is platform dependent type, so it makes no sense at all comparing JIT and interpreted results

[clayborg]

lldb/test/API/lang/c/fpeval/TestFPEval.py

33–34

If you look at how I did the expression, the one that runs with "jit_options" calls a function to get the result. When a function is called, it causes the code to not be interpreted since the interpreter doesn't handle function calls., so this would work.

however I don't see a point in doing it. Why not just check result value?

My main concern with emulating the floating point stuff is getting a different value from host based interpretation than we would if we actually ran it on the actual hardware by jitting it.

[labath]

lldb/test/API/lang/c/fpeval/main.c

4–5

I think that was Greg's point. Interpreted and JITted results should provide the same results even for platform dependent types. llvm's APFloat should know enough about the individual types in order to emulate the operations correctly, and lldb should know enough about the target in order to create the appropriate APFloat instance.

[clayborg]

lldb/test/API/lang/c/fpeval/main.c

4–5

Indeed. and if we are going to replace the float/double/long double interpretation, APFloat can handle that all as we must get the types from the ASTContext for the triple that has the right types anyway so this should be easy to make work.

[kpdev42]

Update test case so it compares JIT’ed and interpreted FP division results and also check operations on float type. Patch doesn’t implement long double, because IR interpreter currently doesn’t support instruction argument size of more than 64 bit which includes both fp128 and int128. This deserves a separate patch, I think

lldb/source/Expression/IRInterpreter.cpp

bool AssignToMatchType(lldb_private::Scalar &scalar, llvm::APInt value,
     return true;
   }
 
   bool EvaluateValue(lldb_private::Scalar &scalar, const Value *value,
                      Module &module) {
     const Constant *constant = dyn_cast<Constant>(value);
 
     if (constant) {
+      if (constant->getValueID() == Value::ConstantFPVal) {
+        if (auto *cfp = dyn_cast<ConstantFP>(constant)) {
+          if (cfp->getType()->isDoubleTy())
+            scalar = cfp->getValueAPF().convertToDouble();
+          else if (cfp->getType()->isFloatTy())
+            scalar = cfp->getValueAPF().convertToFloat();
+          else
+            return false;
+          return true;
+        }
+        return false;
+      }
       APInt value_apint;
 
       if (!ResolveConstantValue(value_apint, constant))
         return false;
 
       return AssignToMatchType(scalar, value_apint, value->getType());
     }
 
     lldb::addr_t process_address = ResolveValue(value, module);
     size_t value_size = m_target_data.getTypeStoreSize(value->getType());
 
     lldb_private::DataExtractor value_extractor;
     lldb_private::Status extract_error;
 
     m_execution_unit.GetMemoryData(value_extractor, process_address,
                                    value_size, extract_error);
 
     if (!extract_error.Success())
       return false;
 
     lldb::offset_t offset = 0;
     if (value_size <= 8) {
+      Type *ty = value->getType();
+      if (ty->isDoubleTy()) {
+        scalar = value_extractor.GetDouble(&offset);
+        return true;
+      } else if (ty->isFloatTy()) {
+        scalar = value_extractor.GetFloat(&offset);
+        return true;
+      } else {
         uint64_t u64value = value_extractor.GetMaxU64(&offset, value_size);
         return AssignToMatchType(scalar, llvm::APInt(64, u64value),
                                  value->getType());
       }
+    }
 
     return false;
   }
 
   bool AssignValue(const Value *value, lldb_private::Scalar scalar,
                    Module &module) {
     lldb::addr_t process_address = ResolveValue(value, module);
 
     if (process_address == LLDB_INVALID_ADDRESS)
       return false;
 
     lldb_private::Scalar cast_scalar;
-
+    Type *vty = value->getType();
+    if (vty->isFloatTy() || vty->isDoubleTy()) {
+      cast_scalar = scalar;
+    } else {
       scalar.MakeUnsigned();
       if (!AssignToMatchType(cast_scalar, scalar.UInt128(llvm::APInt()),
                              value->getType()))
         return false;
+    }
 
     size_t value_byte_size = m_target_data.getTypeStoreSize(value->getType());
 
     lldb_private::DataBufferHeap buf(value_byte_size, 0);
 
     lldb_private::Status get_data_error;
 
     if (!cast_scalar.GetAsMemoryData(buf.GetBytes(), buf.GetByteSize(),
for (Instruction &ii : bb) {
                     PrintValue(&ii).c_str());
           error.SetErrorToGenericError();
           error.SetErrorString(unsupported_opcode_error);
           return false;
         }
       } break;
       case Instruction::GetElementPtr:
         break;
+      case Instruction::FCmp:
       case Instruction::ICmp: {
-        ICmpInst *icmp_inst = dyn_cast<ICmpInst>(&ii);
+        CmpInst *cmp_inst = dyn_cast<CmpInst>(&ii);
 
-        if (!icmp_inst) {
+        if (!cmp_inst) {
           error.SetErrorToGenericError();
           error.SetErrorString(interpreter_internal_error);
           return false;
         }
 
-        switch (icmp_inst->getPredicate()) {
+        switch (cmp_inst->getPredicate()) {
         default: {
           LLDB_LOGF(log, "Unsupported ICmp predicate: %s",
                     PrintValue(&ii).c_str());
 
           error.SetErrorToGenericError();
           error.SetErrorString(unsupported_opcode_error);
           return false;
         }
+        case CmpInst::FCMP_OEQ:
         case CmpInst::ICMP_EQ:
+        case CmpInst::FCMP_UNE:
         case CmpInst::ICMP_NE:
+        case CmpInst::FCMP_OGT:
         case CmpInst::ICMP_UGT:
+        case CmpInst::FCMP_OGE:
         case CmpInst::ICMP_UGE:
+        case CmpInst::FCMP_OLT:
         case CmpInst::ICMP_ULT:
+        case CmpInst::FCMP_OLE:
         case CmpInst::ICMP_ULE:
         case CmpInst::ICMP_SGT:
         case CmpInst::ICMP_SGE:
         case CmpInst::ICMP_SLT:
         case CmpInst::ICMP_SLE:
           break;
         }
       } break;
for (Instruction &ii : bb) {
       case Instruction::Store:
       case Instruction::Sub:
       case Instruction::Trunc:
       case Instruction::UDiv:
       case Instruction::URem:
       case Instruction::Xor:
       case Instruction::ZExt:
         break;
+      case Instruction::FAdd:
+      case Instruction::FSub:
+      case Instruction::FMul:
+      case Instruction::FDiv:
+        break;
       }
 
       for (unsigned oi = 0, oe = ii.getNumOperands(); oi != oe; ++oi) {
         Value *operand = ii.getOperand(oi);
         Type *operand_type = operand->getType();
 
         switch (operand_type->getTypeID()) {
         default:
while (frame.m_ii != frame.m_ie && (++num_insts < 4096)) {
     case Instruction::UDiv:
     case Instruction::SRem:
     case Instruction::URem:
     case Instruction::Shl:
     case Instruction::LShr:
     case Instruction::AShr:
     case Instruction::And:
     case Instruction::Or:
-    case Instruction::Xor: {
+    case Instruction::Xor:
+    case Instruction::FAdd:
+    case Instruction::FSub:
+    case Instruction::FMul:
+    case Instruction::FDiv: {
       const BinaryOperator *bin_op = dyn_cast<BinaryOperator>(inst);
 
       if (!bin_op) {
         LLDB_LOGF(
             log,
             "getOpcode() returns %s, but instruction is not a BinaryOperator",
             inst->getOpcodeName());
         error.SetErrorToGenericError();
case Instruction::FDiv: {
       }
 
       lldb_private::Scalar result;
 
       switch (inst->getOpcode()) {
       default:
         break;
       case Instruction::Add:
+      case Instruction::FAdd:
         result = L + R;
         break;
       case Instruction::Mul:
+      case Instruction::FMul:
         result = L * R;
         break;
       case Instruction::Sub:
+      case Instruction::FSub:
         result = L - R;
         break;
       case Instruction::SDiv:
         L.MakeSigned();
         R.MakeSigned();
         result = L / R;
         break;
       case Instruction::UDiv:
         L.MakeUnsigned();
         R.MakeUnsigned();
         result = L / R;
         break;
+      case Instruction::FDiv:
+        result = L / R;
+        break;
       case Instruction::SRem:
         L.MakeSigned();
         R.MakeSigned();
         result = L % R;
         break;
       case Instruction::URem:
         L.MakeUnsigned();
         R.MakeUnsigned();
case Instruction::GetElementPtr: {
 
       if (log) {
         LLDB_LOGF(log, "Interpreted a GetElementPtrInst");
         LLDB_LOGF(log, "  P       : %s",
                   frame.SummarizeValue(pointer_operand).c_str());
         LLDB_LOGF(log, "  Poffset : %s", frame.SummarizeValue(inst).c_str());
       }
     } break;
+    case Instruction::FCmp:
     case Instruction::ICmp: {
-      const ICmpInst *icmp_inst = cast<ICmpInst>(inst);
+      const CmpInst *icmp_inst = cast<CmpInst>(inst);
 
       CmpInst::Predicate predicate = icmp_inst->getPredicate();
 
       Value *lhs = inst->getOperand(0);
       Value *rhs = inst->getOperand(1);
 
       lldb_private::Scalar L;
       lldb_private::Scalar R;
case Instruction::ICmp: {
       }
 
       lldb_private::Scalar result;
 
       switch (predicate) {
       default:
         return false;
       case CmpInst::ICMP_EQ:
+      case CmpInst::FCMP_OEQ:
         result = (L == R);
         break;
       case CmpInst::ICMP_NE:
+      case CmpInst::FCMP_UNE:
         result = (L != R);
         break;
       case CmpInst::ICMP_UGT:
         L.MakeUnsigned();
         R.MakeUnsigned();
         result = (L > R);
         break;
       case CmpInst::ICMP_UGE:
         L.MakeUnsigned();
         R.MakeUnsigned();
         result = (L >= R);
         break;
+      case CmpInst::FCMP_OGE:
+        result = (L >= R);
+        break;
+      case CmpInst::FCMP_OGT:
+        result = (L > R);
+        break;
       case CmpInst::ICMP_ULT:
         L.MakeUnsigned();
         R.MakeUnsigned();
         result = (L < R);
         break;
+      case CmpInst::FCMP_OLT:
+        result = (L < R);
+        break;
       case CmpInst::ICMP_ULE:
         L.MakeUnsigned();
         R.MakeUnsigned();
         result = (L <= R);
         break;
+      case CmpInst::FCMP_OLE:
+        result = (L <= R);
+        break;
       case CmpInst::ICMP_SGT:
         L.MakeSigned();
         R.MakeSigned();
         result = (L > R);
         break;
       case CmpInst::ICMP_SGE:
         L.MakeSigned();
         R.MakeSigned();

lldb/test/API/lang/c/fpeval/Makefile

+C_SOURCES := main.c
+
+include Makefile.rules

lldb/test/API/lang/c/fpeval/TestFPEval.py

+"""Tests IR interpreter handling of basic floating point operations (fadd, fsub, etc)."""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class FPEvalTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+        self.jit_opts = lldb.SBExpressionOptions()
+        self.jit_opts.SetAllowJIT(True)
+        self.no_jit_opts = lldb.SBExpressionOptions()
+        self.no_jit_opts.SetAllowJIT(False)
+        # Find the line number to break inside main().
+        self.line = line_number('main.c', '// Set break point at this line.')
+
+    def test(self):
+        """Test floating point expressions while jitter is disabled."""
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+        self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+        # Break inside the main.
+        lldbutil.run_break_set_by_file_and_line(
+            self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
+
+        
+        value = self.frame().EvaluateExpression("a + b", self.no_jit_opts)
+
+        self.runCmd("run", RUN_SUCCEEDED)
+        # test double
+        self.expect("expr --allow-jit false  -- a + b", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['double', '44'])
+        self.expect("expr --allow-jit false  -- a - b", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['double', '40'])
+        self.expect("expr --allow-jit false  -- a / b", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['double', '21'])
+        self.expect("expr --allow-jit false  -- a * b", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['double', '84'])
+        self.expect("expr --allow-jit false  -- a + 2", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['double', '44'])
+        self.expect("expr --allow-jit false  -- a > b", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['true'])
+        self.expect("expr --allow-jit false  -- a >= b", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['true'])
+        self.expect("expr --allow-jit false  -- a < b", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['false'])
+        self.expect("expr --allow-jit false  -- a <= b", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['false'])
+        self.expect("expr --allow-jit false  -- a == b", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['false'])
+        self.expect("expr --allow-jit false  -- a != b", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['true'])
+        
+        # test single
+        self.expect("expr --allow-jit false  -- f + q", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['float', '44'])
+        self.expect("expr --allow-jit false  -- f - q", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['float', '40'])
+        self.expect("expr --allow-jit false  -- f / q", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['float', '21'])
+        self.expect("expr --allow-jit false  -- f * q", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['float', '84'])
+        self.expect("expr --allow-jit false  -- f + 2", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['float', '44'])
+        self.expect("expr --allow-jit false  -- f > q", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['true'])
+        self.expect("expr --allow-jit false  -- f >= q", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['true'])
+        self.expect("expr --allow-jit false  -- f < q", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['false'])
+        self.expect("expr --allow-jit false  -- f <= q", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['false'])
+        self.expect("expr --allow-jit false  -- f == q", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['false'])
+        self.expect("expr --allow-jit false  -- f != q", VARIABLES_DISPLAYED_CORRECTLY,
+                    substrs=['true'])
+
+        # compare jit and interpreter output
+        self.assertTrue(self.process().IsValid())
+        thread = lldbutil.get_stopped_thread(self.process(), lldb.eStopReasonBreakpoint)
+        self.assertTrue(thread.IsValid())
+        frame = thread.GetSelectedFrame()
+        self.assertTrue(frame.IsValid())
+       
+        dividents = [42, 79, 666]
+        divisors = [1.618, 2.718281828, 3.1415926535, 6.62607015]
+
+        for x in dividents:
+            for y in divisors:
+                vardef = "double x = {0}, y = {1};".format(x, y) 
+                v1 = frame.EvaluateExpression("{0}; eval(x, y, 2)".format(vardef), self.jit_opts)
+                v2 = frame.EvaluateExpression("{0}; x / y".format(vardef), self.no_jit_opts)
+                self.assertTrue(v1.IsValid() and v2.IsValid())
+                self.assertTrue(str(v1.GetData()) == str(v2.GetData()))
+

lldb/test/API/lang/c/fpeval/main.c

+double eval(double a, double b, int op) {
+  switch (op) {
+  case 0: return a+b;
+  case 1: return a-b;
+  case 2: return a/b;
+  case 3: return a*b;
+  default: return 0;
+  }
+}
+
+int main (int argc, char const *argv[])
+{
+    double a = 42.0, b = 2.0;
+    float f = 42.0, q = 2.0;
+    return 0; //// Set break point at this line.
+}