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.

Sign up for GitHub

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

Jump to bottom

3-component triple *-none-* is incorrectly normalized to *-none-unknown-* instead of *-unknown-none-* #89582

Closed
wzssyqa opened this issue Apr 22, 2024 · 3 comments · Fixed by #89638
Closed

3-component triple *-none-* is incorrectly normalized to *-none-unknown-* instead of *-unknown-none-* #89582

wzssyqa opened this issue Apr 22, 2024 · 3 comments · Fixed by #89638
Assignees
@wzssyqa
Labels
llvm Umbrella label for LLVM issues

Comments

@wzssyqa
Copy link
Contributor

wzssyqa commented Apr 22, 2024 •
edited 

$ clang --target=aarch64-none-elf --print-target-triple
aarch64-none-unknown-elf
$ clang --target=armv7m-none-eabi --print-target-triple
armv7m-none-unknown-eabi

The expected value is that none and unknown should be swapped.
Caused by std::string Triple::normalize(StringRef Str) in llvm/lib/TargetParser/Triple.cpp.
@wzssyqa wzssyqa self-assigned this Apr 22, 2024
@wzssyqa
Copy link
Contributor Author

wzssyqa commented Apr 22, 2024

@peterwaller-arm

@peterwaller-arm
Copy link
Contributor

Is the title of this ticket correct? The issue is that Vendor=none and OS=unknown when triples for example aarch64-none-elf go through normalization, where I believe we expect Vendor=unknown and OS=none.

@wzssyqa wzssyqa changed the title Triple is normalized to unexpected value OS:none, ABI:elf Triple is normalized to unexpected value for 3-section triple with OS:none Apr 22, 2024
wzssyqa added a commit to wzssyqa/llvm-project that referenced this issue Apr 22, 2024
@wzssyqa
Triple::normalize: Set OS for 3-component triple with none as middle
fcac153 
If the middle component of a 3-component triple fails to parse as known
Arch/Vendor/OS/Env, it will fallback as Vendor. While for some cases,
we may wish to recognize it as OS, such as `arm64-none-elf`.

In this patch, we will set OS as `none`, if:
	1) Arch is found;
	2) Env is found;
	3) OS is not found and thus is set as empty;
	4) Vendor is not found while is set as "none",
we will swap Component[2] and Component[3].

Fixes: llvm#89582.
@wzssyqa wzssyqa mentioned this issue Apr 22, 2024
Merged
wzssyqa added a commit to wzssyqa/llvm-project that referenced this issue Apr 22, 2024
@wzssyqa
Triple::normalize: Set OS for 3-component triple with none as middle
8dbb0c7 
If the middle component of a 3-component triple fails to parse as known
Arch/Vendor/OS/Env, it will fallback as Vendor. While for some cases,
we may wish to recognize it as OS, such as `arm64-none-elf`.

In this patch, we will set OS as `none`, if:
	1) Arch is found;
	2) Env is found;
	3) OS is not found and thus is set as empty;
	4) Vendor is not found while is set as "none",
we will swap Component[2] and Component[3].

libcxx/utils/ci/run-buildbot: Use this new triple.

Fixes: llvm#89582.
wzssyqa added a commit to wzssyqa/llvm-project that referenced this issue Apr 23, 2024
@wzssyqa
Triple::normalize: Set OS for 3-component triple with none as middle
a40bf92 
If the middle component of a 3-component triple fails to parse as known
Arch/Vendor/OS/Env, it will fallback as Vendor. While for some cases,
we may wish to recognize it as OS, such as `arm64-none-elf`.

In this patch, we will set OS as `none`, if:
	1) Arch is found;
	2) Env is found;
	3) OS is not found and thus is set as empty;
	4) Vendor is not found while is set as "none",
we will swap Component[2] and Component[3].

Use this new triple for these tests:
  - libcxx/utils/ci/run-buildbot
  - clang/test/Driver/print-multi-selection-flags.c
  - llvm/unittests/TargetParser/TripleTest.cpp

Fixes: llvm#89582.
wzssyqa added a commit to wzssyqa/llvm-project that referenced this issue Apr 23, 2024
@wzssyqa
Triple::normalize: Set OS for 3-component triple with none as middle
b21977e 
If the middle component of a 3-component triple fails to parse as known
Arch/Vendor/OS/Env, it will fallback as Vendor. While for some cases,
we may wish to recognize it as OS, such as `arm64-none-elf`.

In this patch, we will set OS as `none`, if:
	1) Arch is found;
	2) Env is found;
	3) OS is not found and thus is set as empty;
	4) Vendor is not found while is set as "none",
we will swap Component[2] and Component[3].

Use this new triple for these tests:
  - libcxx/utils/ci/run-buildbot
  - clang/test/Driver/arm-triple.c
  - clang/test/Driver/print-multi-selection-flags.c
  - llvm/unittests/TargetParser/TripleTest.cpp

Fixes: llvm#89582.
@wzssyqa wzssyqa changed the title Triple is normalized to unexpected value for 3-section triple with OS:none Triple is normalized to unexpected value for 3-section triple with Middle:none Apr 23, 2024
wzssyqa added a commit to wzssyqa/llvm-project that referenced this issue Apr 23, 2024
@wzssyqa
Triple::normalize: Set OS for 3-component triple with none as middle
11ae27a 
If the middle component of a 3-component triple fails to parse as known
Arch/Vendor/OS/Env, it will fallback as Vendor. While for some cases,
we may wish to recognize it as OS, such as `arm64-none-elf`.

In this patch, we will set OS as `none`, if:
	1) Arch is found;
	2) Env is found;
	3) OS is not found and thus is set as empty;
	4) Vendor is not found while is set as "none",
we will swap Component[2] and Component[3].

Use this new triple for these tests:
  - clang/docs/Multilib.rst
  - clang/test/CodeGen/Inputs/linker-diagnostic1.ll
  - clang/test/CodeGen/linker-diagnostic.ll
  - clang/test/Driver/arm-ias-Wa.s
  - clang/test/Driver/arm-triple.c
  - clang/test/Driver/baremetal-multilib-exclusive-group.yaml
  - clang/test/Driver/baremetal-multilib-group-error.yaml
  - clang/test/Driver/baremetal-multilib-layered.yaml
  - clang/test/Driver/baremetal-multilib.yaml
  - clang/test/Driver/baremetal-sysroot.cpp
  - clang/test/Driver/baremetal.cpp
  - clang/test/Driver/print-multi-selection-flags.c
  - clang/test/Driver/program-path-priority.c
  - clang/test/Preprocessor/init-arm.c
  - clang/unittests/Driver/MultilibTest.cpp
  - clang/unittests/Interpreter/IncrementalCompilerBuilderTest.cpp
  - libcxx/utils/ci/run-buildbot
  - lld/test/ELF/lto/arm.ll
  - llvm/test/CodeGen/ARM/machine-sink-multidef.mir
  - llvm/test/CodeGen/ARM/store-prepostinc.mir
  - llvm/test/CodeGen/ARM/unschedule-reg-sequence.ll
  - llvm/test/CodeGen/Thumb/consthoist-few-dependents.ll
  - llvm/test/CodeGen/Thumb/consthoist-imm8-costs-1.ll
  - llvm/test/CodeGen/Thumb/pr42760.ll
  - llvm/test/CodeGen/Thumb/smul_fix.ll
  - llvm/test/CodeGen/Thumb/smul_fix_sat.ll
  - llvm/test/CodeGen/Thumb/umul_fix.ll
  - llvm/test/CodeGen/Thumb/umul_fix_sat.ll
  - llvm/test/CodeGen/Thumb2/LowOverheadLoops/spillingmove.mir
  - llvm/test/CodeGen/Thumb2/LowOverheadLoops/wls-search-pred.mir
  - llvm/test/CodeGen/Thumb2/high-reg-spill.mir
  - llvm/test/CodeGen/Thumb2/mve-pred-constfold.mir
  - llvm/test/CodeGen/Thumb2/mve-vpt-block-debug.mir
  - llvm/test/CodeGen/Thumb2/pipeliner-preserve-ties.mir
  - llvm/test/CodeGen/Thumb2/store-prepostinc.mir
  - llvm/test/Transforms/InferFunctionAttrs/norecurse_debug.ll
  - llvm/test/Transforms/LoopVectorize/ARM/mve-hoist-runtime-checks.ll
  - llvm/unittests/TargetParser/TripleTest.cpp

Fixes: llvm#89582.
wzssyqa added a commit to wzssyqa/llvm-project that referenced this issue Apr 23, 2024
@wzssyqa
Triple::normalize: Set OS for 3-component triple with none as middle
99978a2 
If the middle component of a 3-component triple fails to parse as known
Arch/Vendor/OS/Env, it will fallback as Vendor. While for some cases,
we may wish to recognize it as OS, such as `arm64-none-elf`.

In this patch, we will set OS as `none`, if:
	1) Arch is found;
	2) Env is found;
	3) OS is not found and thus is set as empty;
	4) Vendor is not found while is set as "none",
we will swap Component[2] and Component[3].

Use this new triple for these tests:
  - clang/docs/Multilib.rst
  - clang/test/CodeGen/Inputs/linker-diagnostic1.ll
  - clang/test/CodeGen/linker-diagnostic.ll
  - clang/test/Driver/arm-ias-Wa.s
  - clang/test/Driver/arm-triple.c
  - clang/test/Driver/baremetal-multilib-exclusive-group.yaml
  - clang/test/Driver/baremetal-multilib-group-error.yaml
  - clang/test/Driver/baremetal-multilib-layered.yaml
  - clang/test/Driver/baremetal-multilib.yaml
  - clang/test/Driver/baremetal-sysroot.cpp
  - clang/test/Driver/baremetal.cpp
  - clang/test/Driver/print-multi-selection-flags.c
  - clang/test/Preprocessor/init-arm.c
  - clang/unittests/Driver/MultilibTest.cpp
  - clang/unittests/Interpreter/IncrementalCompilerBuilderTest.cpp
  - libcxx/utils/ci/run-buildbot
  - lld/test/ELF/lto/arm.ll
  - llvm/test/CodeGen/ARM/machine-sink-multidef.mir
  - llvm/test/CodeGen/ARM/store-prepostinc.mir
  - llvm/test/CodeGen/ARM/unschedule-reg-sequence.ll
  - llvm/test/CodeGen/Thumb/consthoist-few-dependents.ll
  - llvm/test/CodeGen/Thumb/consthoist-imm8-costs-1.ll
  - llvm/test/CodeGen/Thumb/pr42760.ll
  - llvm/test/CodeGen/Thumb/smul_fix.ll
  - llvm/test/CodeGen/Thumb/smul_fix_sat.ll
  - llvm/test/CodeGen/Thumb/umul_fix.ll
  - llvm/test/CodeGen/Thumb/umul_fix_sat.ll
  - llvm/test/CodeGen/Thumb2/LowOverheadLoops/spillingmove.mir
  - llvm/test/CodeGen/Thumb2/LowOverheadLoops/wls-search-pred.mir
  - llvm/test/CodeGen/Thumb2/high-reg-spill.mir
  - llvm/test/CodeGen/Thumb2/mve-pred-constfold.mir
  - llvm/test/CodeGen/Thumb2/mve-vpt-block-debug.mir
  - llvm/test/CodeGen/Thumb2/pipeliner-preserve-ties.mir
  - llvm/test/CodeGen/Thumb2/store-prepostinc.mir
  - llvm/test/Transforms/InferFunctionAttrs/norecurse_debug.ll
  - llvm/test/Transforms/LoopVectorize/ARM/mve-hoist-runtime-checks.ll
  - llvm/unittests/TargetParser/TripleTest.cpp

Fixes: llvm#89582.
wzssyqa added a commit to wzssyqa/llvm-project that referenced this issue Apr 27, 2024
@wzssyqa
Triple::normalize: Use none as OS for XX-none-ABI
aafa018 
When we parse 3-components triples, if the Arch and Env have been
parsed successfully, we have to make a choice between Vendor and
OS for the unrecoginzed component. Noramlly it is the middle one.

In the current code, Vendor is choosed, and then OS is fallbacked
to unknown. It is OK for most cases. But if the unrecoginzed
component is `none`, it is expected to be OS instead of Vendor.

Fixes: llvm#89582.
@MaskRay MaskRay changed the title Triple is normalized to unexpected value for 3-section triple with Middle:none 3-component triple *-none-* is incorrectly normalized to *-none-unknown-* instead of *-unknown-none-* May 1, 2024
@MaskRay
Copy link
Member

MaskRay commented May 1, 2024

Is the title of this ticket correct? The issue is that Vendor=none and OS=unknown when triples for example aarch64-none-elf go through normalization, where I believe we expect Vendor=unknown and OS=none.

I've updated the title. Hopefully it's clearer now.

wzssyqa added a commit to wzssyqa/llvm-project that referenced this issue May 1, 2024
@wzssyqa
Triple::normalize: Use none as OS for XX-none-ABI
cd3107e 
When parsing a 3-component triple, after we determine Arch and Env,
if the middle component is "none", treat it as OS instead of Vendor.

Fixes: llvm#89582.
wzssyqa added a commit to wzssyqa/llvm-project that referenced this issue May 1, 2024
@wzssyqa
Triple::normalize: Use none as OS for XX-none-ABI
3cec71e 
When parsing a 3-component triple, after we determine Arch and Env,
if the middle component is "none", treat it as OS instead of Vendor.

Fixes: llvm#89582.
wzssyqa added a commit to wzssyqa/llvm-project that referenced this issue May 1, 2024
@wzssyqa
Triple::normalize: Use none as OS for XX-none-ABI
2315aa6 
When parsing a 3-component triple, after we determine Arch and Env,
if the middle component is "none", treat it as OS instead of Vendor.

Fixes: llvm#89582.
wzssyqa added a commit to wzssyqa/llvm-project that referenced this issue May 1, 2024
@wzssyqa
Triple::normalize: Use none as OS for XX-none-ABI
d8c9a63 
When parsing a 3-component triple, after we determine Arch and Env,
if the middle component is "none", treat it as OS instead of Vendor.

Fixes: llvm#89582.
wzssyqa added a commit that referenced this issue May 2, 2024
@wzssyqa
Triple::normalize: Use none as OS for XX-none-ABI (#89638)
4b75fcf 
When parsing a 3-component triple, after we determine Arch and Env, if
the middle component is "none", treat it as OS instead of Vendor.

See:
https://discourse.llvm.org/t/rfc-baremetal-target-triple-normalization/78524
Fixes: #89582.
@EugeneZelenko EugeneZelenko added llvm Umbrella label for LLVM issues and removed new issue labels May 2, 2024
@Kojoley Kojoley mentioned this issue May 6, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@wzssyqa wzssyqa

Labels
llvm Umbrella label for LLVM issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Triple::normalize: Use none as OS for XX-none-ABI wzssyqa/llvm-project
4 participants
@wzssyqa @MaskRay @EugeneZelenko @peterwaller-arm