Bring aarch64/codec.txt and aarch64/codec.c in order. #3067
Conversation
This patch adds support for the instructions in the Armv8-a 'Floating-point data-processing' encoding groups with 1 source. Issue #2626 Change-Id: I5b4538490a161fd550396627db3f0478e95b1841
This patch adds support for the instructions in the Armv8-a 'Floating-point data-processing' encoding groups with 2 sources. Issue #2626 Change-Id: If1dd5b3f15daabb0bea405be210da9c71904ec85
This patch adds support for the instructions in the Armv8-a 'Floating-point data-processing' encoding groups with 3 sources. Issue #2626 Change-Id: I58112fee6bdfa7227b29cee09fc96eb7138284a7
This is a cleanup patch order the operands in codec.txt and codec.c by bit-patterns, as originally intended. It also adds a script to check the order, using perl, sort and diff, which is run as part of the encoder/decoder generation. Change-Id: Iaf0b2a38040ef7684c48e6be687eb28bcb27097e
Change-Id: Ia436ef33008aee12724bcaa1170a0612ff41075f
make/CMake_aarch64_gen_codec.cmake
Outdated
|
|
||
| set(CODEC_TXT "${PROJECT_SOURCE_DIR}/core/arch/${ARCH_NAME}/codec.txt") | ||
|
|
||
| execute_process(COMMAND "${PROJECT_SOURCE_DIR}/make/aarch64_check_codec_order.sh" ${PROJECT_SOURCE_DIR}/core/arch/aarch64 ${PROJECT_BINARY_DIR} |
There was a problem hiding this comment.
We can't rely on bash being available on Windows -- but I guess this will never be run on Windows. I think a if (WIN32) message(FATAL_ERROR... would make this clear.
make/CMake_aarch64_gen_codec.cmake
Outdated
| OUTPUT_VARIABLE CHECK_MSG) | ||
|
|
||
| if (CHECK_RC) | ||
| message(FATAL_ERROR "Wrong order in codec.txt or codec.c: ${CHECK_MSG}") |
There was a problem hiding this comment.
style: 2-space indent
| 1ff789e4 : fnmsub h4, h15, h23, h2 : fnmsub %h15 %h23 %h2 -> %h4 |
There was a problem hiding this comment.
What changed on this line?
There was a problem hiding this comment.
I used Github's editor to do a merge, which seems to have messed up the line ending of the last line here.
make/aarch64_check_codec_order.sh
Outdated
| # Usage: ./aarch64_codec_codec_order.sh <path to core/arch/aarch64/> <path to build dir> | ||
|
|
||
| # Check if operand patterns are ordered by pattern. | ||
| cat $1/codec.txt | \ |
There was a problem hiding this comment.
Hmm, when we moved to CMake we wanted to avoid any dependencies on UNIX tools. We got rid of all the uses of sed, awk, grep, cat, cut, head, tail, wc, bash, etc., replacing them with CMake. That made everything cross-platform. (We wanted to get rid of perl too but never finished that: #68). So in that sense this seems like a step backward. I know it is a pain to write file and regex utilities in cmake, and you probably don't want to do that. Please at least include a comment at the top of this file acknowledging that this is not cross-platform, that this file should not be taken as a model for how to write new build utilities for DR, and list the assumptions it makes on all these utility apps being available.
There was a problem hiding this comment.
(Normally CMake would search for and validate utility apps, rather than a blind attempt to execute them.)
|
Thanks Derek. I know the script is not ideal. I'll add a note to the script and I could also add some cmake checks to only run the script if all required binaries are available. What do you think? |
Sure, sounds like a plan. |
script. Change-Id: Iaba2c239e634e19308a226a7158634ad00de28c8
|
I've replaced the bash script with a simple python script, which I think is also more readable. |
| @@ -0,0 +1,88 @@ | |||
| # ********************************************************** | |||
There was a problem hiding this comment.
Perhaps a #! line would be useful, in case someone's running it from the command line. But I'm not sure what it should be:
$ head -n 1 `find -name '*.py'`
==> ./make/upload.py <==
#!/usr/bin/env python
==> ./core/arch/aarch64/codec.py <==
#!/usr/bin/python
==> ./tools/libdynamorio.so-gdb.py <==
#!/usr/bin/env python
There was a problem hiding this comment.
I think #!/usr/bin/env python has the advantage that it will pick the right Python interpreter if you are in a virtualenv. I'll add it (and also make the file executable)
Change-Id: If4766cdf6da7a7205b12147d9b057dd47f161c1d
This is a cleanup patch order the operands in codec.txt and codec.c by
bit-patterns, as originally intended.
It also adds a script to check the order, using perl, sort and diff,
which is run as part of the encoder/decoder generation.