[PATCH] D134638: [Clang][LoongArch] Add inline asm support for constraints k/m/ZB/ZC

[Lu Weining via Phabricator via cfe-commits]

SixWeining added inline comments.


================
Comment at: clang/lib/Basic/Targets/LoongArch.cpp:117
+    // parsing.
+    R = std::string("^") + std::string(Constraint, 2);
+    Constraint++;
----------------
MaskRay wrote:
> First `std::string` can be omitted
Thanks.


================
Comment at: llvm/lib/Target/LoongArch/LoongArchAsmPrinter.cpp:76
+  // TODO: handle extra code.
+  if (!ExtraCode) {
+    const MachineOperand &BaseMO = MI->getOperand(OpNo);
----------------
rengolin wrote:
> NIT: Use early exit.
> 
>     if (ExtraCode)
>       return false;
Thanks. But we should return `true` but not `false`.


================
Comment at: llvm/test/CodeGen/LoongArch/inline-asm-constraint-ZC.ll:16
+;
+; LA64-LABEL: ZC_offset_neg_32769:
+; LA64:       # %bb.0:
----------------
rengolin wrote:
> Same comment as before, these CHECK lines look identical for both targets...
Unfortunately this is not the case becuase different instructions like `add.w`/`add.d` are used by LA32 and LA64 respectively.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134638