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

tools/configure.c: Add missing k210 support. #108

Merged
merged 1 commit into from Jan 16, 2020
Merged

tools/configure.c: Add missing k210 support. #108

merged 1 commit into from Jan 16, 2020

Conversation

patacongo
Copy link
Contributor

Fix a minor but necessary addition to configure.c for the K210 port that was missed in commit e33fc3d

Fix a minor but necessary addition to configure.c for the K210 port that was missed in commit e33fc3d
@xiaoxiang781216
Copy link
Contributor

Fix a minor but necessary addition to configure.c for the K210 port that was missed in commit e33fc3d

It's better to get rid of the hardode arch/board list from tools directory at all, I don't think it's a good design that any addition in arch need to modify so many script in tools folder.

@patacongo
Copy link
Contributor Author

That is a good enhancement request, but I think not relevant here. This change just corrects an error in configure.c. It needs to be merged now. That enhancement can be done later.

Are you arguing that this should not be merged? I don't think that is a wise argument. We need to fix the error. You should open an issue in the NuttX repo for this.

@xiaoxiang781216
Copy link
Contributor

That is a good enhancement request, but I think not relevant here. This change just corrects an error in configure.c. It needs to be merged now. That enhancement can be done later.

Are you arguing that this should not be merged? I don't think that is a wise argument. We need to fix the error. You should open an issue in the NuttX repo for this.

No, we need commit this first, then provide a better solution. I will provide one soon.

@patacongo
Copy link
Contributor Author

patacongo commented Jan 16, 2020

@xiaoxiang781216 Thanks for the clarification. There are three of us doing commits, Alan, Abdelatif, and myself. We normally try to review the changes, run nxstyle, and get the changes to master as quickly as possible. But if there is discussion, then we hold off merging until the discussion reaches consensus.

@jerpelea
Copy link
Contributor

the change is relevant as it is and we should remove the hardcoded path in a future commit

@acassis acassis merged commit 177c037 into apache:master Jan 16, 2020
@acassis
Copy link
Contributor

acassis commented Jan 16, 2020

Done!

@patacongo patacongo deleted the configure branch January 16, 2020 16:47
anchao added a commit to anchao/nuttx that referenced this pull request Dec 1, 2022
…umb code

The option '-mthumb' is only valid for C source files and it is not passed to the assembler.

If 'thumb' is not considered in some assembly projects, the system will generate
'undefined instructions' when running incompatible instruction:

arm_undefinedinsn: Undefined instruction at 0x380cfc98

This assembly file should be compiled with .thumb but it doesn't:

380cfc90 <hobot_i8_i32_gemm_nn_m4_n8_neon>:
380cfc90: e92d4ff0  push  {r4, r5, r6, r7, r8, r9, sl, fp, lr}
380cfc94: ed2d8b10  vpush {d8-d15}
380cfc98: e59d4064  ldr r4, [sp, apache#100]  ; 0x64   <-- Undefined instruction
380cfc9c: e59d5068  ldr r5, [sp, apache#104]  ; 0x68
380cfca0: e59d606c  ldr r6, [sp, apache#108]  ; 0x6c
380cfca4: e59d7070  ldr r7, [sp, apache#112]  ; 0x70
380cfca8: e1a08120  lsr r8, r0, #2
380cfcac: e1a091a1  lsr r9, r1, #3
380cfcb0: e1a0a122  lsr sl, r2, #2

After enable thumb:
  .syntax unified
  .thumb
or
  -Wa,-mthumb

.Lhobot_i8_i32_gemm_nn_m4_n8_neon:
38001100: e92d 4ff0   stmdb sp!, {r4, r5, r6, r7, r8, r9, sl, fp, lr}
38001104: ed2d 8b10   vpush {d8-d15}
38001108: 9c19        ldr r4, [sp, apache#100]  ; 0x64
3800110a: 9d1a        ldr r5, [sp, apache#104]  ; 0x68
3800110c: 9e1b        ldr r6, [sp, apache#108]  ; 0x6c
3800110e: 9f1c        ldr r7, [sp, apache#112]  ; 0x70
38001110: ea4f 0890   mov.w r8, r0, lsr #2
38001114: ea4f 09d1   mov.w r9, r1, lsr #3
38001118: ea4f 0a92   mov.w sl, r2, lsr #2

This commit will enable the thumb option of the assembly file by default,
so that when compiling the assembly file, the machine code and the system will be in a consistent state.

----------------------------------------------------------------
https://gcc.gnu.org/onlinedocs/gcc-4.5.2/gcc/ARM-Options.html

GCC Manual:
-mthumb
  Generate code for the Thumb instruction set. The default is to use the 32-bit ARM
  instruction set. This option automatically enables either 16-bit Thumb-1 or mixed 16/32-bit
  Thumb-2 instructions based on the -mcpu=name and -march=name options.

  ** This option is not passed to the assembler. **
  ** If you want to force assembler files to be interpreted as Thumb code,
     either add a `.thumb' directive to the source or pass the -mthumb option
     directly to the assembler by prefixing it with -Wa. **

Signed-off-by: chao an <anchao@xiaomi.com>
pkarashchenko pushed a commit that referenced this pull request Dec 1, 2022
…umb code

The option '-mthumb' is only valid for C source files and it is not passed to the assembler.

If 'thumb' is not considered in some assembly projects, the system will generate
'undefined instructions' when running incompatible instruction:

arm_undefinedinsn: Undefined instruction at 0x380cfc98

This assembly file should be compiled with .thumb but it doesn't:

380cfc90 <hobot_i8_i32_gemm_nn_m4_n8_neon>:
380cfc90: e92d4ff0  push  {r4, r5, r6, r7, r8, r9, sl, fp, lr}
380cfc94: ed2d8b10  vpush {d8-d15}
380cfc98: e59d4064  ldr r4, [sp, #100]  ; 0x64   <-- Undefined instruction
380cfc9c: e59d5068  ldr r5, [sp, #104]  ; 0x68
380cfca0: e59d606c  ldr r6, [sp, #108]  ; 0x6c
380cfca4: e59d7070  ldr r7, [sp, #112]  ; 0x70
380cfca8: e1a08120  lsr r8, r0, #2
380cfcac: e1a091a1  lsr r9, r1, #3
380cfcb0: e1a0a122  lsr sl, r2, #2

After enable thumb:
  .syntax unified
  .thumb
or
  -Wa,-mthumb

.Lhobot_i8_i32_gemm_nn_m4_n8_neon:
38001100: e92d 4ff0   stmdb sp!, {r4, r5, r6, r7, r8, r9, sl, fp, lr}
38001104: ed2d 8b10   vpush {d8-d15}
38001108: 9c19        ldr r4, [sp, #100]  ; 0x64
3800110a: 9d1a        ldr r5, [sp, #104]  ; 0x68
3800110c: 9e1b        ldr r6, [sp, #108]  ; 0x6c
3800110e: 9f1c        ldr r7, [sp, #112]  ; 0x70
38001110: ea4f 0890   mov.w r8, r0, lsr #2
38001114: ea4f 09d1   mov.w r9, r1, lsr #3
38001118: ea4f 0a92   mov.w sl, r2, lsr #2

This commit will enable the thumb option of the assembly file by default,
so that when compiling the assembly file, the machine code and the system will be in a consistent state.

----------------------------------------------------------------
https://gcc.gnu.org/onlinedocs/gcc-4.5.2/gcc/ARM-Options.html

GCC Manual:
-mthumb
  Generate code for the Thumb instruction set. The default is to use the 32-bit ARM
  instruction set. This option automatically enables either 16-bit Thumb-1 or mixed 16/32-bit
  Thumb-2 instructions based on the -mcpu=name and -march=name options.

  ** This option is not passed to the assembler. **
  ** If you want to force assembler files to be interpreted as Thumb code,
     either add a `.thumb' directive to the source or pass the -mthumb option
     directly to the assembler by prefixing it with -Wa. **

Signed-off-by: chao an <anchao@xiaomi.com>
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.

None yet

5 participants