[PATCH] D156444: [llvm][RISCV][IR] Zext flag in IR for RISC-V

Panagiotis K via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 31 14:04:16 PDT 2023


karouzakisp added a comment.

In D156444#4624088 <https://reviews.llvm.org/D156444#4624088>, @nikic wrote:

> This is still missing LangRef changes.
>
> I think you should also drop everything here that is related to supporting zext constant expressions. These will be removed in the near future, and I don't think it makes sense to add nneg support for them just to drop them again.

"
Semantics:

The zext fills the high order bits of the value with zero bits until it reaches the size of the destination type, ty2.

When zero extending from i1, the result will always be either 0 or 1.

The nneg flag means the value to be zero extended is non negative.
So we can safely convert the zext to a sext.
This applies only for the RISC-V target because sext i32 to i64 can be a no op for RV64
and it is always cheaper than performing a zero extension .

Example:
%X = zext i32 257 to i64              ; yields i64:257
%Y = zext i1 true to i32              ; yields i32:1
%Z = zext <2 x i16> <i16 8, i16 7> to <2 x i32> ; yields <i32 8, i32 7>
%H = zext nneg 8 i32 to i64        ; yields to i64:8



================
Comment at: llvm/lib/AsmParser/LLParser.cpp:6355
       return true;
+
     if (Exact) cast<BinaryOperator>(Inst)->setIsExact(true);
----------------
craig.topper wrote:
> Unrelated change?
Yes, I reverted it back as it was.


================
Comment at: llvm/lib/AsmParser/LLParser.cpp:6382
+  case lltok::kw_zext: {
+    bool WasSext = EatIfPresent(lltok::kw_was_sext);
+    bool Res = parseCast(Inst, PFS, KeywordVal);
----------------
craig.topper wrote:
> Can this be moved into parseCast?
It can be moved but I wrote this, as all the other flags was written in a similar way. Also if we move it to parseCast we must refactor parseCast with one additional optional argument.


================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:4874
       if (OpNum < Record.size()) {
-        if (Opc == Instruction::Add ||
-            Opc == Instruction::Sub ||
-            Opc == Instruction::Mul ||
-            Opc == Instruction::Shl) {
-          if (Record[OpNum] & (1 << bitc::OBO_NO_SIGNED_WRAP))
-            cast<BinaryOperator>(I)->setHasNoSignedWrap(true);
+        if (Opc == Instruction::Add || Opc == Instruction::Sub ||
+            Opc == Instruction::Mul || Opc == Instruction::Shl) {
----------------
craig.topper wrote:
> Is this reformatting existing code that you aren't changing?
Probably 
git clang-format
reformatted it. 
Should I reformat it back?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156444



More information about the llvm-commits mailing list