[PATCH] D134798: [LoongArch] Do not assert value type in isFPImmLegal

[WÁNG Xuěruì via Phabricator via llvm-commits]

xen0n added inline comments.


================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:1813
                                            bool ForCodeSize) const {
-  assert((VT == MVT::f32 || VT == MVT::f64) && "Unexpected VT");
-
----------------
Thinking about it harder, do we want to inspect the element type of vectors and ensure corresponding ISA extension is present? In which case another similar assert is needed/better.


================
Comment at: llvm/test/CodeGen/LoongArch/vector-fp-imm.ll:7
+
+;; TODO: Merge the offset of address calculation into the offset field of instructions.
+
----------------
gonglingqin wrote:
> xen0n wrote:
> > What part of the expected output does this comment refer to? I can only see the offsets like 4 8 or 12 nicely placed inside the `f{ld,st}`'s already...
> For line 142-143 of this file, the expected output is `fld.s $fa0, $a2, %pc_lo12(.LCPI1_0)`.  After the functionality is complete, similar scenarios can be optimized by adding a backend pass.
OK I see. Although normally it would be better to clarify the comment here, considering it is going to be a short-lived one anyway I'm fine with it as-is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134798