This is an archive of the discontinued LLVM Phabricator instance.

Change signature of __builtin_rotateright64 back to unsigned
ClosedPublic

Authored by Ka-Ka on Sep 16 2019, 1:09 AM.

Diff Detail

Event Timeline

Ka-Ka created this revision.Sep 16 2019, 1:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 16 2019, 1:09 AM
Herald added a subscriber: kristina. · View Herald Transcript
hans added a comment.EditedSep 16 2019, 1:27 AM

A simpler way to test this might be to check for conversion warnings in the existing clang/test/CodeGen/avr-builtins.c test, for example as below. I think that's better since it covers more of the signatures, e.g. the rotateleft ones too.

diff --git a/clang/test/CodeGen/avr-builtins.c b/clang/test/CodeGen/avr-builtins.c
index cbba6b2f2a2..0d9ce91ad69 100644
--- a/clang/test/CodeGen/avr-builtins.c
+++ b/clang/test/CodeGen/avr-builtins.c
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 -triple avr-unknown-unknown -emit-llvm -o - %s | FileCheck %s
 
+// Check that the parameter types match.
+// RUN: %clang_cc1 -triple avr-unknown-unknown -Wconversion -verify %s
+// expected-no-diagnostics
+
 unsigned char bitrev8(unsigned char data) {
     return __builtin_bitreverse8(data);
 }
Ka-Ka updated this revision to Diff 220294.Sep 16 2019, 2:10 AM

Update testcases according to review comment.

hans accepted this revision.Sep 16 2019, 2:25 AM

Looks good to me! (with comment)

test/CodeGen/avr-builtins.c
3

Sorry, I didn't put this in my example, but maybe also mention the Bug number here.

This revision is now accepted and ready to land.Sep 16 2019, 2:25 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 16 2019, 2:50 AM