Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core(A): align compiler header files to core(M) implementation #128

Merged

Conversation

Masmiseim36
Copy link
Contributor

Both versions (Cortex-M and Cortex-A) are better comparable now.

Compare also ARM-software/CMSIS_5#1587

CMSIS/Core/Include/a-profile/cmsis_gcc_a.h Dismissed Show dismissed Hide dismissed
Copy link

github-actions bot commented Jan 28, 2024

Test Results

0 files   -    348  0 suites   - 348   0s ⏱️ - 8m 5s
0 tests  -     49  0 ✅  -     44  0 💤  -     5  0 ❌ ±0 
0 runs   - 17 052  0 ✅  - 11 076  0 💤  - 5 976  0 ❌ ±0 

Results for commit 3c18518. ± Comparison against base commit d52f8b7.

♻️ This comment has been updated with latest results.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omit an immediate offset of 0.
In my opinion it is better to omit an immediate offset of 0 for unprivileged STR instructions.
The offset is also not used for GCC nor for unprivileged LDR instructions.

@JonatanAntoni
Copy link
Member

JonatanAntoni commented Jan 29, 2024

The Core Checks failed.
https://github.com/ARM-software/CMSIS_6/actions/runs/7687108827/job/20946766300?pr=128

Changing strt results in different assembly output. If this is intentional, we'd need to update the tests as well.

Mapping to ACLE intrinsics in Cortex-A requires additional include of


#if (__ARM_ACLE >= 200)
  #include <arm_acle.h>
#else
  #error Compiler must support ACLE V2.0
#endif /* (__ARM_ACLE >= 200) */

@Masmiseim36
Copy link
Contributor Author

Hello,

I am currently struggling to run the automatic validation test locally. In the file CMSIS\Core\Test\lit.cfg.py the environment variables

  • AC6_TOOLCHAIN_
  • CLANG_TOOLCHAIN_
  • GCC_TOOLCHAIN_

are required. Unfortunately, I don't know what value these should be defined with? Is it the full path to the corresponding development environment?

@ghost
Copy link

ghost commented Feb 9, 2024

Hello @Masmiseim36,
the environment variables hold the location of the toolchain's bin folder. e.g.:

  • AC6_TOOLCHAIN_ C:\_Tools\ArmCompilerforEmbedded6.20\bin

@JonatanAntoni
Copy link
Member

Hi @Masmiseim36,

Appreciated, this detail is not given in the README.md for manual installation of the tools. Once you are using vcpkg to install all tools as listed in vcpkg-configuration.json these env variables are set automatically.

As @GuentherMartin already mentioned, these shall point to the bin folders of the according toolchain as defined in CMSIS-Build, see https://learn.arm.com/install-guides/cmsis-toolbox/.

@Masmiseim36
Copy link
Contributor Author

Hi @Masmiseim36,

Appreciated, this detail is not given in the README.md for manual installation of the tools. Once you are using vcpkg to install all tools as listed in vcpkg-configuration.json these env variables are set automatically.

As @GuentherMartin already mentioned, these shall point to the bin folders of the according toolchain as defined in CMSIS-Build, see https://learn.arm.com/install-guides/cmsis-toolbox/.

Hello @JonatanAntoni and @GuentherMartin

I had used vcpkg as described in the readme, but in my case it raises an error after install. Maybe this is the part where the environment variable should have been registerd:

vcpkg activate
warning: vcpkg-artifacts is experimental and may change at any time.
Artifact                            Version Status    Dependency Summary
arm:compilers/arm/armclang 6.21.0  installed            Arm Compiler for Embedded
arm:compilers/arm/arm-none-eabi-gcc 13.2.1  installed            GCC compiler for ARM CPUs.
arm:compilers/arm/llvm-embedded 17.0.1  installed            LLVM Embedded Toolchain for Arm

arm:compilers/arm/arm-none-eabi-gcc - EULA: https://developer.arm.com/GetEula?Id=37988a7c-c40e-4b78-9fd1-62c20b507aa8
error: no postscript file: rerun with the vcpkg shell function rather than executable

When using Linux I get the following error message:

vcpkg activate
warning: vcpkg-artifacts is experimental and may change at any time.
Downloading standalone bundle 2024-02-07.
A suitable version of node was not found (required v16.15.1) Downloading portable node 16.15.1...
Downloading node...
https://nodejs.org/dist/v16.15.1/node-v16.15.1-linux-x64.tar.gz->/home/markus/Schreibtisch/vcpkg/downloads/node-v16.15.1-linux-x64.tar.gz
Extracting node...
Updating registry data from https://github.com/microsoft/vcpkg-ce-catalog/archive/refs/heads/main.zip
Updating registry data from https://artifacts.tools.arm.com/vcpkg-registry
/mnt/vss/_work/1/s/src/vcpkg/metrics.cpp(166): unreachable code was reached
node:internal/process/promises:279
            triggerUncaughtException(err, true /* fromPromise */);
            ^

[UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "Running vcpkg internally returned a nonzero exit code: null".] {
  code: &apos;ERR_UNHANDLED_REJECTION&apos;
}

Which Version of vpkg are you using? I used 2024-02-07-8a83681f921b10d86ae626fd833c253f4f8c355b

However, I got the test-run working on latest CMSIS6-main branch for GCC and CLANG. AC6 is only working for M0(+) Core all, other runs fail with exit code 1. When executing the test manually I get

lit.EXE --xunit-xml-output lit.xml -D toolchain=AC6 -D device=CM3 -D optimize=none src
-- Testing: 49 tests, 32 workers --
UNSUPPORTED: CMSIS-Core :: src/simd.c (1 of 49)
UNSUPPORTED: CMSIS-Core :: src/cp15.c (2 of 49)
UNSUPPORTED: CMSIS-Core :: src/sp.c (3 of 49)
UNSUPPORTED: CMSIS-Core :: src/fpexc.c (4 of 49)
UNSUPPORTED: CMSIS-Core :: src/fpscr.c (5 of 49)
UNSUPPORTED: CMSIS-Core :: src/cpsr.c (6 of 49)
UNSUPPORTED: CMSIS-Core :: src/msplim.c (7 of 49)
UNSUPPORTED: CMSIS-Core :: src/sp_ns.c (8 of 49)
UNSUPPORTED: CMSIS-Core :: src/psplim_baseline.c (9 of 49)
UNSUPPORTED: CMSIS-Core :: src/lda.c (10 of 49)
FAIL: CMSIS-Core :: src/sev.c (11 of 49)
UNSUPPORTED: CMSIS-Core :: src/fpexc_nofp.c (12 of 49)
UNSUPPORTED: CMSIS-Core :: src/psplim.c (13 of 49)
UNSUPPORTED: CMSIS-Core :: src/ldaex.c (14 of 49)
FAIL: CMSIS-Core :: src/rrx.c (15 of 49)
UNSUPPORTED: CMSIS-Core :: src/stl.c (16 of 49)
UNSUPPORTED: CMSIS-Core :: src/stlex.c (17 of 49)
UNSUPPORTED: CMSIS-Core :: src/systick.c (18 of 49)
FAIL: CMSIS-Core :: src/clrex.c (19 of 49)
FAIL: CMSIS-Core :: src/isb.c (20 of 49)
FAIL: CMSIS-Core :: src/strex.c (21 of 49)
PASS: CMSIS-Core :: src/basepri.c (22 of 49)
FAIL: CMSIS-Core :: src/dsb.c (23 of 49)
FAIL: CMSIS-Core :: src/nop.c (24 of 49)
FAIL: CMSIS-Core :: src/fpscr_nofp.c (25 of 49)
FAIL: CMSIS-Core :: src/clz.c (26 of 49)
FAIL: CMSIS-Core :: src/noreturn.c (27 of 49)
PASS: CMSIS-Core :: src/primask.c (28 of 49)
FAIL: CMSIS-Core :: src/bkpt.c (29 of 49)
PASS: CMSIS-Core :: src/xpsr.c (30 of 49)
PASS: CMSIS-Core :: src/msp.c (31 of 49)
PASS: CMSIS-Core :: src/faultmask.c (32 of 49)
FAIL: CMSIS-Core :: src/ldrex.c (33 of 49)
FAIL: CMSIS-Core :: src/rev.c (34 of 49)
FAIL: CMSIS-Core :: src/sat.c (35 of 49)
FAIL: CMSIS-Core :: src/wfi.c (36 of 49)
PASS: CMSIS-Core :: src/ipsr.c (37 of 49)
FAIL: CMSIS-Core :: src/ldrt.c (38 of 49)
PASS: CMSIS-Core :: src/psp.c (39 of 49)
FAIL: CMSIS-Core :: src/dmb.c (40 of 49)
PASS: CMSIS-Core :: src/control.c (41 of 49)
FAIL: CMSIS-Core :: src/rev16.c (42 of 49)
FAIL: CMSIS-Core :: src/irq.c (43 of 49)
FAIL: CMSIS-Core :: src/revsh.c (44 of 49)
FAIL: CMSIS-Core :: src/rbit.c (45 of 49)
FAIL: CMSIS-Core :: src/strt.c (46 of 49)
FAIL: CMSIS-Core :: src/ror.c (47 of 49)
FAIL: CMSIS-Core :: src/fault_irq.c (48 of 49)
PASS: CMSIS-Core :: src/apsr.c (49 of 49)
********************
Failed Tests (24):
  CMSIS-Core :: src/bkpt.c
  CMSIS-Core :: src/clrex.c
  CMSIS-Core :: src/clz.c
  CMSIS-Core :: src/dmb.c
  CMSIS-Core :: src/dsb.c
  CMSIS-Core :: src/fault_irq.c
  CMSIS-Core :: src/fpscr_nofp.c
  CMSIS-Core :: src/irq.c
  CMSIS-Core :: src/isb.c
  CMSIS-Core :: src/ldrex.c
  CMSIS-Core :: src/ldrt.c
  CMSIS-Core :: src/nop.c
  CMSIS-Core :: src/noreturn.c
  CMSIS-Core :: src/rbit.c
  CMSIS-Core :: src/rev.c
  CMSIS-Core :: src/rev16.c
  CMSIS-Core :: src/revsh.c
  CMSIS-Core :: src/ror.c
  CMSIS-Core :: src/rrx.c
  CMSIS-Core :: src/sat.c
  CMSIS-Core :: src/sev.c
  CMSIS-Core :: src/strex.c
  CMSIS-Core :: src/strt.c
  CMSIS-Core :: src/wfi.c


Testing Time: 6.32s
  Unsupported: 16
  Passed     :  9
  Failed     : 24

As said, Cortex-M0(+) tests are working with AC6 the others seem to create 'UNSUPPORTED: CMSIS-Core' messages. GCC and CLANG are okay with any architecture. Any Idea?

Best Regards

@JonatanAntoni
Copy link
Member

JonatanAntoni commented Feb 12, 2024

I had used vcpkg as described in the readme, but in my case it raises an error after install. Maybe this is the part where the environment variable should have been registerd:

error: no postscript file: rerun with the vcpkg shell function rather than executable

This seems to be an issue of vcpkg usage on command line is unclear, probably due to lack of documentation. How did you install vcpkg?

The "problem" here is how to use vcpkg on command line for getting the development environment configured. The way it works (see the workflow files as reference) is documented at https://github.com/Open-CMSIS-Pack/cmsis-toolbox/blob/main/docs/installation.md#vcpkg---setup-using-cli (originally from https://github.com/microsoft/vcpkg-tool/blob/main/README.md). I.e., one need to source the vcpkg-init script into the current shell environment. This makes vcpkg actually a shell function rather than calling the executable directly (refer to error message above). This is required to allow vcpkg to modify the current shell environment. Take note, running an executable creates a subprocess which cannot modify it's parent processes environment.

Further issues we have with vcpkg is its limited range of supported environments. It currently doesn't work on aarch64 linux. And on Windows it works on Command or Powershell but not from an MSYS Bash. And, it seems like running vcpkg in a virtual machine (I tested on a Mac using Parallels) there are issues with downloading files.

However, I got the test-run working on latest CMSIS6-main branch for GCC and CLANG. AC6 is only working for M0(+) Core all, other runs fail with exit code 1. When executing the test manually I get

lit.EXE --xunit-xml-output lit.xml -D toolchain=AC6 -D device=CM3 -D optimize=none src

This doesn't look too bad. At least you got something running. Try to run single tests with more verbose output to see what's actually going wrong:

lit -D toolchain=AC6 -D device=CM3 -D optimize=none -a src/sev.c

@ghost
Copy link

ghost commented Feb 12, 2024

Hello @Masmiseim36,
it looks like that the immediate offset of 0 which you introduced for some instructions causes problem.

$ lit -D toolchain=AC6 -D device=CM3 -D optimize=none -a src/strt.c
-- Testing: 1 tests, 1 workers --
FAIL: CMSIS-Core :: src/strt.c (1 of 1)
******************** TEST 'CMSIS-Core :: src/strt.c' FAILED ********************
Script:
--
: 'RUN: at line 2';   C:\_Tools\ArmCompilerforEmbedded6.20\bin\armclang --target=arm-arm-none-eabi -mcpu=cortex-m3 -mfpu=none -O1 -I C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Include -c -D CORE_HEADER="core_
cm3.h" -D __CM3_REV=0x0000U -D __MPU_PRESENT=1U -D __VTOR_PRESENT=1U -D __NVIC_PRIO_BITS=3U -D __Vendor_SysTickConfig=0U -o C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Test\src\Output/strt.c.o C:\_Repos\GitHub
\xxx\CMSIS_6\CMSIS\Core\Test\src\strt.c; C:\_Tools\LLVMEmbeddedToolchainForArm-17.0.1\bin\llvm-objdump --mcpu=cortex-m3 -d C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Test\src\Output/strt.c.o | FileCheck --all
ow-unused-prefixes --check-prefixes CHECK,CHECK-THUMB C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Test\src\strt.c
--
Exit Code: 2

Command Output (stdout):
--
$ ":" "RUN: at line 2"
$ "C:\_Tools\ArmCompilerforEmbedded6.20\bin\armclang" "--target=arm-arm-none-eabi" "-mcpu=cortex-m3" "-mfpu=none" "-O1" "-I" "C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Include" "-c" "-D" "CORE_HEADER=core_cm
3.h" "-D" "__CM3_REV=0x0000U" "-D" "__MPU_PRESENT=1U" "-D" "__VTOR_PRESENT=1U" "-D" "__NVIC_PRIO_BITS=3U" "-D" "__Vendor_SysTickConfig=0U" "-o" "C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Test\src\Output/strt
.c.o" "C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Test\src\strt.c"
# command stderr:
In file included from C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Test\src\strt.c:4:
In file included from C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Include\cmsis_compiler.h:37:
C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Include/./m-profile/cmsis_armclang_m.h:506:19: error: too many operands for instruction
  __ASM volatile ("strbt %1, %0, #0" : "=Q" (*ptr) : "r" ((uint32_t)value) );
                  ^
<inline asm>:1:18: note: instantiated into assembly here
        strbt r1, [r0], #0
                        ^
In file included from C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Test\src\strt.c:4:
In file included from C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Include\cmsis_compiler.h:37:
C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Include/./m-profile/cmsis_armclang_m.h:518:19: error: too many operands for instruction
  __ASM volatile ("strht %1, %0, #0" : "=Q" (*ptr) : "r" ((uint32_t)value) );
                  ^
<inline asm>:1:18: note: instantiated into assembly here
        strht r1, [r0], #0
                        ^
In file included from C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Test\src\strt.c:4:
In file included from C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Include\cmsis_compiler.h:37:
C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Include/./m-profile/cmsis_armclang_m.h:530:19: error: too many operands for instruction
  __ASM volatile ("strt %1, %0, #0" : "=Q" (*ptr) : "r" (value) );
                  ^
<inline asm>:1:17: note: instantiated into assembly here
        strt r1, [r0], #0
                       ^
3 errors generated.

error: command failed with exit status: 1
$ "C:\_Tools\LLVMEmbeddedToolchainForArm-17.0.1\bin\llvm-objdump" "--mcpu=cortex-m3" "-d" "C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Test\src\Output/strt.c.o"
# command stderr:
C:\_Tools\LLVMEmbeddedToolchainForArm-17.0.1\bin\llvm-objdump.exe: error: 'C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Test\src\Output/strt.c.o': no such file or directory

error: command failed with exit status: 1
$ "FileCheck" "--allow-unused-prefixes" "--check-prefixes" "CHECK,CHECK-THUMB" "C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Test\src\strt.c"
# command stderr:
FileCheck error: '<stdin>' is empty.
FileCheck command line:  filecheck.exe --allow-unused-prefixes --check-prefixes CHECK,CHECK-THUMB C:\_Repos\GitHub\xxx\CMSIS_6\CMSIS\Core\Test\src\strt.c

error: command failed with exit status: 2

--

********************
********************
Failed Tests (1):
  CMSIS-Core :: src/strt.c

Testing Time: 8.47s
Failed: 1

@Masmiseim36
Copy link
Contributor Author

Hello @Masmiseim36, it looks like that the immediate offset of 0 which you introduced for some instructions causes problem.

Hello @GuentherMartin,

I am currently using the unmodified main version of the repository. I need a reference before I look at what problems my changes are causing.

The problem seems to be somewhere in the test framework. In all likelihood, I am doing something wrong. The only question is what.

Best Regards
Markus

@Masmiseim36
Copy link
Contributor Author

Hello @JonatanAntoni

This seems to be an issue of vcpkg usage on command line is unclear, probably due to lack of documentation. How did you install vcpkg?

I have installed it as described in https://github.com/microsoft/vcpkg

git clone https://github.com/microsoft/vcpkg
.\vcpkg\bootstrap-vcpkg.bat

This doesn't look too bad. At least you got something running. Try to run single tests with more verbose output to see what's actually going wrong:

lit -D toolchain=AC6 -D device=CM3 -D optimize=none -a src/sev.c

Got it, I need a license for AC6 :-(

armclang: error: Failed to check out a license.
The license file could not be found. Check that ARMLMD_LICENSE_FILE is set correctly.
ARMLMD_LICENSE_FILE is not set. You must set this to the path to your license.
Information about this error is available at https://developer.arm.com/support/lic87/m1
 General licensing information is available at https://developer.arm.com/support/licensing/

@Masmiseim36
Copy link
Contributor Author

Omit an immediate offset of 0.
In my opinion it is better to omit an immediate offset of 0 for unprivileged STR instructions.
The offset is also not used for GCC nor for unprivileged LDR instructions.

The whole thing is a bit strange. Clang generates a compiler error if the offset is omitted on an ARMv7-A architecture. With ARMv7-M, on the other hand, an error is generated if it is present. In both cases, the offset is optional according to the specification. This looks like a bug in Clang to me.

@ghost
Copy link

ghost commented Feb 13, 2024

It is strange.
According ARMv7-M Architecture Reference Manual an immediate offset in the range of 0..255 is allowed.

Assembler syntax
STRT<c><q> <Rt>, [<Rn> {, #<imm>}]
where:
<c><q>  See Standard assembler syntax fields on page A7-177.
<Rt>    Specifies the source register.
<Rn>    Specifies the base register. This register is permitted to be the SP.
<imm>   Specifies the immediate offset added to the value of <Rn> to form the address. The range of permitted
        values is 0-255. <imm> can be omitted, meaning an offset of 0.

But if it is used then AC6, CLang and GCC throw an error.
Maybe I have overseen something in the documentation.

@ReinhardKeil
Copy link
Contributor

I would argue that the correct syntax is:

strt r1, [r0, #0]

Currently this gets expanded to

strt r1, [r0], #0

Not sure what needs to change in the __ASM definition to achieve that.

@JonatanAntoni
Copy link
Member

I have installed it as described in https://github.com/microsoft/vcpkg

Yes, this is the installation for "vcpkg packages" but we are using "vcpkg artifacts". I appreciate, this is not easy to understand for first time users.

Got it, I need a license for AC6 :-(

Ah, okay. In this case, I think it's fine to use the Arm MDK6 Community license. Try this:

armlm activate --server https://mdk-preview.keil.arm.com --product KEMDK-COM0

Regarding the assembly syntax, there might be some slight difference between ARM and THUMB instruction sets in how the encoding needs to look like. Let me check and I'll report back later. But, I think this confirms why having these kind of tests is a good thing.

@JonatanAntoni
Copy link
Member

JonatanAntoni commented Feb 13, 2024

THUMB and ARM encodings are indeed different, check
https://developer.arm.com/documentation/ddi0406/c/Application-Level-Architecture/Instruction-Details/Alphabetical-list-of-instructions/STRT?lang=en.

In THUMB it is

STRT<c> <Rt>, [<Rn>, #<imm8>]

And in ARM it is

STRT<c> <Rt>, [<Rn>] {, +/-<imm12>}

It looks like GCC is tolerant to different formats but Clang is more strict.

@Masmiseim36
Copy link
Contributor Author

THUMB and ARM encodings are indeed different, check https://developer.arm.com/documentation/ddi0406/c/Application-Level-Architecture/Instruction-Details/Alphabetical-list-of-instructions/STRT?lang=en.

In THUMB it is

STRT<c> <Rt>, [<Rn>, #<imm8>]

And in ARM it is

STRT<c> <Rt>, [<Rn>] {, +/-<imm12>}

It looks like GCC is tolerant to different formats but Clang is more strict.

Interesting. However, STRHT and STRBT appear to be defined identically.

THUMB:

STRHT<c><q>  <Rt>, [<Rn> {, #<imm>}]
STRBT<c><q>  <Rt>, [<Rn> {, #<imm>}]

https://developer.arm.com/documentation/ddi0403/d/Application-Level-Architecture/Instruction-Details/Alphabetical-list-of-ARMv7-M-Thumb-instructions/STRHT

ARM

STRHT{<c>}{<q>}  <Rt>, [<Rn> {, #<imm>}]
STRBT{<c>}{<q>}  <Rt>, [<Rn> {, #<imm>}]

https://developer.arm.com/documentation/ddi0406/c/Application-Level-Architecture/Instruction-Details/Alphabetical-list-of-instructions/STRHT?lang=en

@JonatanAntoni
Copy link
Member

We need to be very careful about the encoding used. On Cortex-M we only have Thumb instruction set while on Cortex-A you can use Thumb or Arm instruction set. The Arm instruction encoding differs from the Thumb one.

Now, I do not understand why you want to add the implicit immediate value #0 explicitly into all the inline assembly. Another issue is the omission of [ ] around the second operand.

@Masmiseim36 Masmiseim36 force-pushed the feature/Align_aprofile2mprofile branch from 31e13ab to 3c18518 Compare February 14, 2024 12:32
@Masmiseim36
Copy link
Contributor Author

We need to be very careful about the encoding used. On Cortex-M we only have Thumb instruction set while on Cortex-A you can use Thumb or Arm instruction set. The Arm instruction encoding differs from the Thumb one.

Now, I do not understand why you want to add the implicit immediate value #0 explicitly into all the inline assembly. Another issue is the omission of [ ] around the second operand.

In the pull request, I try to bring the different compiler and architecture abstractions closer together. In my local tests, the version with an additional offset worked with Clang and GCC. I cannot currently explain why my test behaved differently to the official test suite.

I have reversed the change as I have no other solution in the short term.

@JonatanAntoni
Copy link
Member

In the pull request, I try to bring the different compiler and architecture abstractions closer together.

I think we are aligned here, please take this on.

In my local tests, the version with an additional offset worked with Clang and GCC.

Yes, this should work, of course. Functional wise there should be no difference between omitting the #<imm> part or provide #0 (which is the implicit default).

Hence, I'd expect the binary value in the object file to be identical for both cases. That makes me wondering how the disassembler could ever create different disassembly output.

I cannot currently explain why my test behaved differently to the official test suite.

Let me do some other experiments on my side.
Could it be a different version of llvm-objdump used for disassembling the object file?

I have reversed the change as I have no other solution in the short term.

Okay, then perhaps we can address the refactorings in smaller steps.

@JonatanAntoni JonatanAntoni merged commit 13e9f0c into ARM-software:main Feb 14, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants