Skip to content

Conversation

@NikitaRudenkoIntel
Copy link
Contributor

@NikitaRudenkoIntel NikitaRudenkoIntel commented Jun 3, 2020

Extensions are published at intel/llvm#1612 and intel/llvm#1611

@AlexeySotkin
Copy link
Contributor

In general the PR looks good to me. It is marked as draft. Are there anything else to be done? If not I suggest to push "Ready for review" button.

Extension is published at intel/llvm#1612

Co-Authored-By: Aleksandr Bezzubikov  <[email protected]>
Co-Authored-By: Aleksander Us         <[email protected]>
Co-Authored-By: Alexey Sachkov        <[email protected]>
Co-Authored-By: Alexey Sotkin         <[email protected]>
Co-Authored-By: Anton Sidorenko       <[email protected]>
Co-Authored-By: Gang Chen             <[email protected]>
Co-Authored-By: Kai Chen              <[email protected]>
Co-Authored-By: Konstantin Vladimirov <[email protected]>
Co-Authored-By: Wei Pan               <[email protected]>

Change-Id: Ic8bd645316f054729b8571efd2b9800a38cb723c
@NikitaRudenkoIntel NikitaRudenkoIntel marked this pull request as ready for review June 10, 2020 10:44
Copy link
Contributor

@AlexeySachkov AlexeySachkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor comments. Other than that, looks good to me

F->addAttribute(AttributeList::FunctionIndex, Attr);
}

if (F->getCallingConv() != CallingConv::SPIR_KERNEL)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the point of this if if we anyway return true below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An artifact of previous revisions. Fixed.

// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
// Copyright (c) 2018 Intel Corporation. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Copyright (c) 2018 Intel Corporation. All rights reserved.
// Copyright (c) 2020 Intel Corporation. All rights reserved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
// Copyright (c) 2018 Intel Corporation. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Copyright (c) 2018 Intel Corporation. All rights reserved.
// Copyright (c) 2020 Intel Corporation. All rights reserved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

#include "SPIRVInternal.h"
#include "SPIRVUtil.h"
#include "spirv.hpp"
#include "llvm/IR/Module.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#include "llvm/IR/Module.h"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@@ -0,0 +1,32 @@
; RUN: llvm-as %s -o %t.bc
; RUN: llvm-spirv %t.bc -o %t.spv --spirv-ext=+SPV_INTEL_vector_compute --spirv-ext=+SPV_KHR_float_controls --spirv-ext=+SPV_INTEL_float_controls2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
; RUN: llvm-spirv %t.bc -o %t.spv --spirv-ext=+SPV_INTEL_vector_compute --spirv-ext=+SPV_KHR_float_controls --spirv-ext=+SPV_INTEL_float_controls2
; RUN: llvm-spirv %t.bc -o %t.spv --spirv-ext=+SPV_INTEL_vector_compute,+SPV_KHR_float_controls,+SPV_INTEL_float_controls2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@@ -0,0 +1,36 @@
; RUN: llvm-as %s -o %t.bc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these tests are performing both direct and reverse translation, could you please move them into transcoding folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Backend part

Extension is published at intel/llvm#1612

Co-Authored-By: Aleksandr Bezzubikov  <[email protected]>
Co-Authored-By: Aleksander Us         <[email protected]>
Co-Authored-By: Alexey Sachkov        <[email protected]>
Co-Authored-By: Alexey Sotkin         <[email protected]>
Co-Authored-By: Anton Sidorenko       <[email protected]>
Co-Authored-By: Gang Chen             <[email protected]>
Co-Authored-By: Kai Chen              <[email protected]>
Co-Authored-By: Konstantin Vladimirov <[email protected]>
Co-Authored-By: Wei Pan               <[email protected]>

Change-Id: Ic8bd645316f054729b8571efd2b9800a38cb723c
Frontend part

Extension is published at intel/llvm#1612

Co-Authored-By: Aleksandr Bezzubikov  <[email protected]>
Co-Authored-By: Aleksander Us         <[email protected]>
Co-Authored-By: Alexey Sachkov        <[email protected]>
Co-Authored-By: Alexey Sotkin         <[email protected]>
Co-Authored-By: Anton Sidorenko       <[email protected]>
Co-Authored-By: Gang Chen             <[email protected]>
Co-Authored-By: Kai Chen              <[email protected]>
Co-Authored-By: Konstantin Vladimirov <[email protected]>
Co-Authored-By: Wei Pan               <[email protected]>

Change-Id: Ic8bd645316f054729b8571efd2b9800a38cb723c
SPV_INTEL_float_controls2 and SPV_KHR_float_controls extensions

Extensions are published at intel/llvm#1612
                            intel/llvm#1611
@svenvh svenvh removed their request for review June 10, 2020 12:56
@AlexeySotkin AlexeySotkin merged commit 4c37246 into KhronosGroup:master Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants