This is an archive of the discontinued LLVM Phabricator instance.

[VE] vaarg functions callers and callees
ClosedPublic

Authored by simoll on Jan 30 2020, 7:39 AM.

Details

Summary

Isel patterns and tests for vaarg functions as callers and callees.

Diff Detail

Event Timeline

simoll created this revision.Jan 30 2020, 7:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 30 2020, 7:39 AM

Unit tests: fail. 62342 tests passed, 2 failed and 842 were skipped.

failed: libc++.std/containers/sequences/array/array_creation/to_array.fail.cpp
failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_sharedtimedmutex_requirements/thread_sharedtimedmutex_class/try_lock.pass.cpp

clang-tidy: fail. clang-tidy found 4 errors and 6 warnings. 0 of them are added as review comments below (why?).

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

arsenm added inline comments.Jan 30 2020, 8:24 AM
llvm/lib/Target/VE/VEInstrInfo.cpp
328

Braces

353

Braces

llvm/test/CodeGen/VE/va_arg2.ll
24 ↗(On Diff #241452)

There's too much going on in one test function for my taste

simoll planned changes to this revision.Jan 30 2020, 8:48 AM
simoll marked an inline comment as done.

Thanks for looking into this!

llvm/test/CodeGen/VE/va_arg2.ll
24 ↗(On Diff #241452)

Fair enough. I'll break this up into tests for:

  • va_arg function call
  • va_copy
  • fpext load
  • fptrunc store
simoll updated this revision to Diff 241664.Jan 31 2020, 2:49 AM
simoll marked 2 inline comments as done.
  • addressed comments
  • split up tests
  • lint fixes
  • rebased

I will move the fptruncstore , fpextload changes&tests to a separate commit

Unit tests: pass. 62358 tests passed, 0 failed and 844 were skipped.

clang-tidy: fail. clang-tidy found 4 errors and 6 warnings. 0 of them are added as review comments below (why?).

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

simoll updated this revision to Diff 241688.Jan 31 2020, 4:02 AM
simoll updated this revision to Diff 241690.Jan 31 2020, 4:15 AM
  • Undo the format clean up in unrelated code (will do a cleanup commit for VEInstrInfo.cpp after this patch).

Unit tests: pass. 62361 tests passed, 0 failed and 842 were skipped.

clang-tidy: fail. clang-tidy found 4 errors and 6 warnings. 0 of them are added as review comments below (why?).

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

Unit tests: fail. 62360 tests passed, 1 failed and 842 were skipped.

failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_mutex_requirements_mutex/thread_mutex_recursive/try_lock.pass.cpp

clang-tidy: fail. clang-tidy found 4 errors and 6 warnings. 0 of them are added as review comments below (why?).

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

simoll updated this revision to Diff 241724.Jan 31 2020, 6:58 AM
  • cleanup
  • added isLoadFrom/isStoreTo Stack
arsenm added inline comments.Jan 31 2020, 7:14 AM
llvm/lib/Target/VE/VEInstrInfo.cpp
311

Formatting

llvm/lib/Target/VE/VEInstrInfo.h
64–77

These are overdue for conversion to Register

llvm/test/CodeGen/VE/va_callee.ll
81

Missing checks in other functions?

Unit tests: fail. 62368 tests passed, 2 failed and 842 were skipped.

failed: LLVM.CodeGen/AMDGPU/llvm.amdgcn.s.buffer.load.ll
failed: LLVM.CodeGen/AMDGPU/smrd.ll

clang-tidy: fail. clang-tidy found 4 errors and 6 warnings. 0 of them are added as review comments below (why?).

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

simoll updated this revision to Diff 241731.Jan 31 2020, 7:43 AM
simoll marked 3 inline comments as done.
  • addressed comments
  • missing va_callee.ll checks

unsigned -> Register changes planned for follow up patch

llvm/lib/Target/VE/VEInstrInfo.h
64–77

Yep. I'll work on a followup patch for the conversion next week.

Unit tests: pass. 62370 tests passed, 0 failed and 842 were skipped.

clang-tidy: fail. clang-tidy found 4 errors and 6 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

simoll updated this revision to Diff 241991.Feb 3 2020, 1:39 AM

Unit tests: pass. 62407 tests passed, 0 failed and 842 were skipped.

clang-tidy: fail. clang-tidy found 4 errors and 6 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

simoll updated this revision to Diff 242051.Feb 3 2020, 6:16 AM
simoll marked an inline comment as done.
  • rebased onto re-formatted VEInstrInfo code

Unit tests: pass. 62414 tests passed, 0 failed and 842 were skipped.

clang-tidy: fail. clang-tidy found 4 errors and 6 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

arsenm accepted this revision.Feb 3 2020, 7:03 AM

LGTM with nit

llvm/lib/Target/VE/VEInstrInfo.cpp
311

Formatting?

331

Formatting?

This revision is now accepted and ready to land.Feb 3 2020, 7:03 AM
simoll updated this revision to Diff 242068.Feb 3 2020, 7:27 AM
simoll marked 2 inline comments as done.
  • addressed formatting issues
This revision was automatically updated to reflect the committed changes.

Unit tests: pass. 62414 tests passed, 0 failed and 842 were skipped.

clang-tidy: fail. clang-tidy found 4 errors and 6 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.