This is an archive of the discontinued LLVM Phabricator instance.

Differential D78163

[AVR][NFC] Move preprocessor tests to Preprocessor directory
ClosedPublic

Authored by aykevl on Apr 14 2020, 3:50 PM.

Details

Reviewers
dylanmckay
rjmccall
MaskRay
Commits
rGceba881aeac1: [AVR][NFC] Move preprocessor tests to Preprocessor directory
Summary

These tests were placed in the CodeGen directory while they really should have been placed in the Preprocessor directory.

See https://reviews.llvm.org/D78117 for background.

Diff Detail

Event Timeline

aykevl created this revision.Apr 14 2020, 3:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2020, 3:50 PM
Herald added subscribers: cfe-commits, Jim. · View Herald Transcript
MaskRay added a comment.Apr 14 2020, 4:12 PM

Not sure we need target-cpu-defines/

You can add a lit.local.cfg to avoid REQUIRES: avr-registered-target

Harbormaster completed remote builds in B53255: Diff 257541.Apr 14 2020, 4:52 PM
rjmccall added a comment.Apr 14 2020, 8:35 PM

Looks like the established convention would be to just put these tests in test/Preprocessor with an avr- prefix.

aykevl added a comment.Apr 15 2020, 6:50 AM

Honestly I don't know the convention but I agree with @rjmccall that the convention appears to be to not use a separate subdirectory (I don't see any other arch-specific subdirectories). So, should the files be moved to clang/test/Preprocessor/avr-*.c (e.g. clang/test/Preprocessor/avr-attiny104.c) while keeping REQUIRES: avr-registered-target?

rjmccall added a comment.Apr 15 2020, 10:51 AM

I don't think the REQUIRES is needed unless there's something special about AVR as a target. Clang doesn't conditionally compile out frontend target support.

aykevl updated this revision to Diff 259621.Apr 23 2020, 10:18 AM
  • moved test files out of the AVR-specific directory
  • removed the REQUIRES: line
In D78163#1984210, @rjmccall wrote:

I don't think the REQUIRES is needed unless there's something special about AVR as a target. Clang doesn't conditionally compile out frontend target support.

I grepped for REQUIRES: and found just a few files with such a line - most files by far didn't specify any requirements. So it should be fine to leave it out.

rjmccall accepted this revision.Apr 23 2020, 10:30 AM

LGTM, thanks!

This revision is now accepted and ready to land.Apr 23 2020, 10:30 AM
MaskRay accepted this revision.Apr 23 2020, 10:49 AM
Harbormaster failed remote builds in B54434: Diff 259621!Apr 23 2020, 11:53 AM
Closed by commit rGceba881aeac1: [AVR][NFC] Move preprocessor tests to Preprocessor directory (authored by aykevl). · Explain WhyApr 25 2020, 5:00 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
clang/
test/
CodeGen/
avr/
target-cpu-defines/
Preprocessor/
CodeGen/
avr-atmega328p.c/
avr/
target-cpu-defines/
atmega328p.c
1 line
avr-attiny104.c/
avr/
target-cpu-defines/
attiny104.c
1 line
avr-common.c/
avr/
target-cpu-defines/
common.c
1 line

Diff 260139

clang/test/CodeGen/avr/target-cpu-defines/atmega328p.c

  • This file was moved to clang/test/Preprocessor/avr-atmega328p.c.

clang/test/CodeGen/avr/target-cpu-defines/attiny104.c

  • This file was moved to clang/test/Preprocessor/avr-attiny104.c.

clang/test/CodeGen/avr/target-cpu-defines/common.c

  • This file was moved to clang/test/Preprocessor/avr-common.c.

clang/test/Preprocessor/avr-atmega328p.c

  • This file was moved from clang/test/CodeGen/avr/target-cpu-defines/atmega328p.c.
// REQUIRES: avr-registered-target
// RUN: %clang_cc1 -E -dM -triple avr-unknown-unknown -target-cpu atmega328p /dev/null | FileCheck -match-full-lines %s // RUN: %clang_cc1 -E -dM -triple avr-unknown-unknown -target-cpu atmega328p /dev/null | FileCheck -match-full-lines %s
// CHECK: #define AVR 1 // CHECK: #define AVR 1
// CHECK: #define __AVR 1 // CHECK: #define __AVR 1
// CHECK: #define __AVR_ATmega328P__ 1 // CHECK: #define __AVR_ATmega328P__ 1
// CHECK: #define __AVR__ 1 // CHECK: #define __AVR__ 1
// CHECK: #define __ELF__ 1 // CHECK: #define __ELF__ 1

clang/test/Preprocessor/avr-attiny104.c

  • This file was moved from clang/test/CodeGen/avr/target-cpu-defines/attiny104.c.
// REQUIRES: avr-registered-target
// RUN: %clang_cc1 -E -dM -triple avr-unknown-unknown -target-cpu attiny104 /dev/null | FileCheck -match-full-lines %s // RUN: %clang_cc1 -E -dM -triple avr-unknown-unknown -target-cpu attiny104 /dev/null | FileCheck -match-full-lines %s
// CHECK: #define AVR 1 // CHECK: #define AVR 1
// CHECK: #define __AVR 1 // CHECK: #define __AVR 1
// CHECK: #define __AVR_ATtiny104__ 1 // CHECK: #define __AVR_ATtiny104__ 1
// CHECK: #define __AVR__ 1 // CHECK: #define __AVR__ 1
// CHECK: #define __ELF__ 1 // CHECK: #define __ELF__ 1

clang/test/Preprocessor/avr-common.c

  • This file was moved from clang/test/CodeGen/avr/target-cpu-defines/common.c.
// REQUIRES: avr-registered-target
// RUN: %clang_cc1 -E -dM -triple avr-unknown-unknown /dev/null | FileCheck -match-full-lines %s // RUN: %clang_cc1 -E -dM -triple avr-unknown-unknown /dev/null | FileCheck -match-full-lines %s
// CHECK: #define AVR 1 // CHECK: #define AVR 1
// CHECK: #define __AVR 1 // CHECK: #define __AVR 1
// CHECK: #define __AVR__ 1 // CHECK: #define __AVR__ 1
// CHECK: #define __ELF__ 1 // CHECK: #define __ELF__ 1