[PATCH] D88392: [M68k] (Patch 6/8) IR Tests

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/test/CodeGen/M68k/ASM/Arith/imul-neg.ll:4
+; FIXME: When using SelectionDAGISel, the following cases use
+; `sub` rather than the expected `neg`
+
----------------
Please can you raise a bug for these?


================
Comment at: llvm/utils/extract-section.py:1
+#!/usr/bin/env python
+from __future__ import print_function
----------------
myhsu wrote:
> jrtc27 wrote:
> > myhsu wrote:
> > > MaskRay wrote:
> > > > If not used, please drop it.
> > > > 
> > > > `llvm-objcopy --dump-section=.foo=file a.o /dev/null`
> > > If you're referring to this script as a whole, it is used extensively in M68k's test suite. 
> > > 
> > > This script is designed for printing certain section in //textual// format. So I'm afraid your llvm-objcopy command might not be sufficient. As the comment below described, this script is similar to llvm-readobj but the latter only support one output format (i.e. hex with lots of redundant info like ASCii decoding), where in many cases, we want other formats like hex string with different width and textual bits.
> > I'm with MaskRay, this script shouldn't exist, use the existing llvm tools.
> many of the tests here are checking against binary number, but IIRC the tools LLVM currently have are only able to print out hexadecimal number. The only solution (to get rid of this script) I can think of now is to rewrite all the checks in the tests, but that will be a non-trivial amount of works
I have no objections to the extract-section.py as a temporary script but it would most likely have to go before m68k becomes non-experimental. @rengolin ?


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

https://reviews.llvm.org/D88392