LLVM: Support logical binop for strings, int, real #1506
Conversation
| or_op1 = a or b | ||
| or_op2 = x or y |
There was a problem hiding this comment.
I think we should explicitly be casting the operands tobool() (when are not already of bool type). For example:
res: bool
res = bool(a) or bool(b)For or_op1 = a or b, since or_op1 is of type i32, I think it should be
or_op1 = i32(bool(a) or bool(b))There was a problem hiding this comment.
It seems the result of x or y is an integer, I think we are fine in this case.
There was a problem hiding this comment.
It seems the result of x or y is an integer, I think we are fine in this case.
It looks like or here is not a logical operation, but instead a selective operation. I think we should have a separate ASR node for this selective or operation.
There was a problem hiding this comment.
Maybe we can name it as SelectiveOr which takes two arguments both of any type (but equal types). The return value of this SelectiveOr is the same as that of the type of one of the operands.
| llvm::Value *zero, *cond; | ||
| llvm::AllocaInst *result; | ||
| if (ASRUtils::is_integer(*x.m_type)) { | ||
| int a_kind = down_cast<ASR::Integer_t>(x.m_type)->m_kind; | ||
| int init_value_bits = 8*a_kind; | ||
| zero = llvm::ConstantInt::get(context, | ||
| llvm::APInt(init_value_bits, 0)); | ||
| cond = builder->CreateICmpEQ(left_val, zero); | ||
| result = builder->CreateAlloca(getIntType(a_kind), nullptr); | ||
| } else if (ASRUtils::is_real(*x.m_type)) { | ||
| int a_kind = down_cast<ASR::Real_t>(x.m_type)->m_kind; | ||
| int init_value_bits = 8*a_kind; | ||
| if (init_value_bits == 32) { | ||
| zero = llvm::ConstantFP::get(context, | ||
| llvm::APFloat((float)0)); | ||
| } else { | ||
| zero = llvm::ConstantFP::get(context, | ||
| llvm::APFloat((double)0)); | ||
| } | ||
| result = builder->CreateAlloca(getFPType(a_kind), nullptr); | ||
| cond = builder->CreateFCmpUEQ(left_val, zero); | ||
| } else if (ASRUtils::is_character(*x.m_type)) { | ||
| zero = llvm::Constant::getNullValue(character_type); | ||
| cond = lfortran_str_cmp(left_val, zero, "_lpython_str_compare_eq"); | ||
| result = builder->CreateAlloca(character_type, nullptr); | ||
| } else if (ASRUtils::is_logical(*x.m_type)) { | ||
| zero = llvm::ConstantInt::get(context, | ||
| llvm::APInt(1, 0)); | ||
| cond = builder->CreateICmpEQ(left_val, zero); | ||
| result = builder->CreateAlloca(llvm::Type::getInt1Ty(context), nullptr); |
There was a problem hiding this comment.
I think this operation should not be done at the backend level. A backend should exactly follow what the ASR says. In this case, it should always return a logical result. This logical result can/should later be casted to the desired type.
There was a problem hiding this comment.
Opened an issue here lfortran/lfortran#3620.
There was a problem hiding this comment.
Yes, this should be explicitly done in ASR, not in the backend.
Fixes #1487