This is an archive of the discontinued LLVM Phabricator instance.

Fix incorrect target triple in fp16-promote.ll
ClosedPublic

Authored by pirama on Oct 15 2015, 4:47 PM.

Details

Summary

Hyphens were missing from the triple, causing it to be parsed
incorrectly. This patch updates the triple and makes necessary
changes to the expected output.

Patch is from Vinicius Tinti.

Diff Detail

Event Timeline

pirama updated this revision to Diff 37538.Oct 15 2015, 4:47 PM
pirama retitled this revision from to Fix incorrect target triple in fp16-promote.ll.
pirama updated this object.
pirama added reviewers: ab, tinti.
pirama added subscribers: llvm-commits, srhines.
rengolin requested changes to this revision.Oct 16 2015, 6:28 AM
rengolin added a reviewer: rengolin.
rengolin added a subscriber: rengolin.

Hi,

This is weird, and I'd like to know more why the extra dash makes that big a difference...

cheers,
--renato

This revision now requires changes to proceed.Oct 16 2015, 6:28 AM
tinti edited edge metadata.Oct 20 2015, 12:35 PM

Hi,

This is weird, and I'd like to know more why the extra dash makes that big a difference...

It is because the "environment" is not parsed in the original case.
Thus Subtarget->isTargetAnything won't work because the "environment" is set to Unknown.

This is the environment parser:
http://llvm.org/docs/doxygen/html/Triple_8cpp_source.html#l00441

cheers,
--renato

tinti added a comment.Oct 20 2015, 1:42 PM

@rengolin

Frontends emit LLVM IR with --- (three dashes) to avoid ambiguous interpretation. Example:

; ModuleID = 'main.c'
target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
target triple = "armv7---eabihf"

Abiguous triples are only accepted in -target in Clang for compatibility reasons.

tinti accepted this revision.Oct 20 2015, 1:43 PM
tinti edited edge metadata.
rengolin accepted this revision.Oct 20 2015, 1:44 PM
rengolin edited edge metadata.

Frontends emit LLVM IR with --- (three dashes) to avoid ambiguous interpretation. Example:

Indeed! LGTM. Thanks!

This revision is now accepted and ready to land.Oct 20 2015, 1:44 PM
This revision was automatically updated to reflect the committed changes.