Page MenuHomePhabricator

pavelkopyl (Pavel Kopyl)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 21 2022, 4:42 AM (54 w, 5 d)

Recent Activity

Thu, Feb 2

pavelkopyl added a comment to D141737: [NVPTX] Unforce minimum alignment of 4 for byval arguments of device-side functions..

I guess, there is one more reason to getting back to default alignment.It seems we break ABI with alignment forced to 4. This may be an issue for mixed build clang + llvm.

Thu, Feb 2, 3:53 AM · Restricted Project, Restricted Project

Thu, Jan 26

pavelkopyl abandoned D142508: [OpenMP][libomptarget] Fix alignment calculation for mapping struct members..

Proper fix is here: https://reviews.llvm.org/D142586

Thu, Jan 26, 11:21 AM · Restricted Project, Restricted Project
pavelkopyl added a comment to D142508: [OpenMP][libomptarget] Fix alignment calculation for mapping struct members..

Your approach fails the second scenario in the D142586 test case. The problem is you look at the first member mapped but that is not necessarily the first member mapped later.

Thu, Jan 26, 11:20 AM · Restricted Project, Restricted Project
pavelkopyl accepted D142586: [OpenMP][FIX] Do not overalign mapped structures.
Thu, Jan 26, 11:19 AM · Restricted Project, Restricted Project
pavelkopyl added inline comments to D142586: [OpenMP][FIX] Do not overalign mapped structures.
Thu, Jan 26, 11:17 AM · Restricted Project, Restricted Project
pavelkopyl added a comment to D142586: [OpenMP][FIX] Do not overalign mapped structures.

Thank you for the patch! I checked it with the D135462 PR that changes alloca alignment - everything works fine.

Thu, Jan 26, 10:56 AM · Restricted Project, Restricted Project

Wed, Jan 25

pavelkopyl added a comment to D141737: [NVPTX] Unforce minimum alignment of 4 for byval arguments of device-side functions..

Do we need to do it? I mean -- are there any observable differences between enforced alignment of 4 and not? For register-passed parameters it will make no difference, and for the parameters passed via constant/local memory, it may or may not make a difference.

I guess at the extreme it would impact how many 1-byte parameters we may have, as the total size of param address space has an upper bound (for kernels it's 4K: https://docs.nvidia.com/cuda/cuda-c-programming-guide/#parameter-buffer-layout). If that's the case, then it probably does not matter whether we can pass 4K arguments or only 1K.

In other words, I do not see a practical need to introduce an option which may not have any practical use.

Sorry for long response.
OK, I'll remove the option. I just wanted to clarify, are you agree in general with removing this workaround?

Wed, Jan 25, 5:17 PM · Restricted Project, Restricted Project
pavelkopyl updated the summary of D141737: [NVPTX] Unforce minimum alignment of 4 for byval arguments of device-side functions..
Wed, Jan 25, 4:56 PM · Restricted Project, Restricted Project
pavelkopyl updated the diff for D142508: [OpenMP][libomptarget] Fix alignment calculation for mapping struct members..
  • Limit Padding values
Wed, Jan 25, 9:04 AM · Restricted Project, Restricted Project
pavelkopyl added a comment to D142508: [OpenMP][libomptarget] Fix alignment calculation for mapping struct members..

Padding was added to libomptarget in order to ensure 8-byte struct members remain properly aligned, e.g. pointers, doubles etc. If you remove padding, then such a member may find itself in a non 8-aligned address, making its access result in a segfault.

For instance, in the code you provided, add a third struct member k of type double

struct S {
  int i;
  int j;
  double k;
}

and partially map the struct map(s.j, s.k). There are architectures (e.g. CUDA for sure) where memalloc will always return an 8-aligned (or even higher) address. If we don't add a 4-byte padding at the beginning of the struct on the device, j will start at an address of the form 0x...0, then k will find itself at an address of the form 0x...4, i.e. k will not be 8-aligned. Try accessing it on virtually any architecture and you'll get a segfault. By adding 4 dummy bytes at the beginning of the struct, j finds itself at 0x...4 and k at 0x...8, which is what we need to ensure.

Wed, Jan 25, 9:02 AM · Restricted Project, Restricted Project

Tue, Jan 24

pavelkopyl added a comment to D142508: [OpenMP][libomptarget] Fix alignment calculation for mapping struct members..

Why not add the test?

Tue, Jan 24, 3:04 PM · Restricted Project, Restricted Project
pavelkopyl added a comment to D142508: [OpenMP][libomptarget] Fix alignment calculation for mapping struct members..

Is that link correct? It points to a very old and seemingly irrelevant patch.

Tue, Jan 24, 2:53 PM · Restricted Project, Restricted Project
pavelkopyl added a comment to D142508: [OpenMP][libomptarget] Fix alignment calculation for mapping struct members..

Hi All,

Tue, Jan 24, 2:47 PM · Restricted Project, Restricted Project
pavelkopyl updated the summary of D142508: [OpenMP][libomptarget] Fix alignment calculation for mapping struct members..
Tue, Jan 24, 2:36 PM · Restricted Project, Restricted Project
pavelkopyl added reviewers for D142508: [OpenMP][libomptarget] Fix alignment calculation for mapping struct members.: grokos, Hahnfeld, asavonic.
Tue, Jan 24, 2:35 PM · Restricted Project, Restricted Project
pavelkopyl requested review of D142508: [OpenMP][libomptarget] Fix alignment calculation for mapping struct members..
Tue, Jan 24, 2:30 PM · Restricted Project, Restricted Project

Fri, Jan 13

pavelkopyl added a reviewer for D141737: [NVPTX] Unforce minimum alignment of 4 for byval arguments of device-side functions.: tra.
Fri, Jan 13, 4:49 PM · Restricted Project, Restricted Project
pavelkopyl requested review of D141737: [NVPTX] Unforce minimum alignment of 4 for byval arguments of device-side functions..
Fri, Jan 13, 4:48 PM · Restricted Project, Restricted Project
pavelkopyl added a comment to D140436: [NVPTX] Add support of ptxas v12.0 to the LIT tests.

I've prepared a new review where ptxas is run -march=sm_60: https://reviews.llvm.org/D141736

Fri, Jan 13, 3:58 PM · Restricted Project, Restricted Project
pavelkopyl added a reviewer for D141736: [NVPTX] Use 'sm_60' architecture when expanding %ptxas-verify macro.: tra.
Fri, Jan 13, 3:56 PM · Restricted Project, Restricted Project
pavelkopyl requested review of D141736: [NVPTX] Use 'sm_60' architecture when expanding %ptxas-verify macro..
Fri, Jan 13, 3:53 PM · Restricted Project, Restricted Project

Mon, Jan 9

pavelkopyl added a comment to D140436: [NVPTX] Add support of ptxas v12.0 to the LIT tests.

Description could use some details. I.e. what makes sm_35 special? I assume it's due to the fact that sm_35 is no longer supported.

Instead of special-casing ptxas-12.0, perhpas a better way is to just always run ptxas with -arch=sm_60 so it would work with ptxas from all reasonably new CUDA versions.

Mon, Jan 9, 4:59 PM · Restricted Project, Restricted Project
pavelkopyl updated the summary of D140436: [NVPTX] Add support of ptxas v12.0 to the LIT tests.
Mon, Jan 9, 4:28 PM · Restricted Project, Restricted Project
pavelkopyl updated the diff for D140436: [NVPTX] Add support of ptxas v12.0 to the LIT tests.
  • Rebase
Mon, Jan 9, 4:27 PM · Restricted Project, Restricted Project

Jan 7 2023

pavelkopyl added inline comments to D140581: [NVPTX] Enforce minumum alignment of 4 for byval parametrs in a function prototype.
Jan 7 2023, 4:42 PM · Restricted Project, Restricted Project
pavelkopyl updated the diff for D140581: [NVPTX] Enforce minumum alignment of 4 for byval parametrs in a function prototype.
  • Fixes according to review notes
Jan 7 2023, 4:41 PM · Restricted Project, Restricted Project

Jan 6 2023

pavelkopyl added inline comments to D140581: [NVPTX] Enforce minumum alignment of 4 for byval parametrs in a function prototype.
Jan 6 2023, 4:31 PM · Restricted Project, Restricted Project
pavelkopyl updated the diff for D140581: [NVPTX] Enforce minumum alignment of 4 for byval parametrs in a function prototype.
  • Rebased
  • Added hepler function for computing byval param alignments of device functions
Jan 6 2023, 4:24 PM · Restricted Project, Restricted Project

Jan 5 2023

pavelkopyl updated the diff for D141054: [NVPTX] Set default version of architecture to SM_30, PTX to 6.0..
  • Rebase
  • Fixes according to a review notes
Jan 5 2023, 2:43 PM · Restricted Project, Restricted Project
pavelkopyl added inline comments to D141054: [NVPTX] Set default version of architecture to SM_30, PTX to 6.0..
Jan 5 2023, 2:04 PM · Restricted Project, Restricted Project
pavelkopyl updated the diff for D141054: [NVPTX] Set default version of architecture to SM_30, PTX to 6.0..
  • Update to correct patch version.
Jan 5 2023, 8:08 AM · Restricted Project, Restricted Project
pavelkopyl added a comment to D141054: [NVPTX] Set default version of architecture to SM_30, PTX to 6.0..

Example of failing tests: https://lab.llvm.org/buildbot/#/builders/232/builds/2407

Jan 5 2023, 7:11 AM · Restricted Project, Restricted Project
pavelkopyl updated the summary of D141054: [NVPTX] Set default version of architecture to SM_30, PTX to 6.0..
Jan 5 2023, 7:06 AM · Restricted Project, Restricted Project
pavelkopyl requested review of D141054: [NVPTX] Set default version of architecture to SM_30, PTX to 6.0..
Jan 5 2023, 7:04 AM · Restricted Project, Restricted Project

Dec 30 2022

pavelkopyl updated the summary of D140754: [NFC][Assignment Tracking Analysis] Add x86 triple to lower-offset-expression.ll.
Dec 30 2022, 3:57 PM · Restricted Project, Restricted Project
pavelkopyl updated the diff for D140754: [NFC][Assignment Tracking Analysis] Add x86 triple to lower-offset-expression.ll.

-Updated commit message

Dec 30 2022, 3:55 PM · Restricted Project, Restricted Project

Dec 29 2022

pavelkopyl added a comment to D140581: [NVPTX] Enforce minumum alignment of 4 for byval parametrs in a function prototype.

In your commit message:

  • s/parametrs/parameters

I'd prefer to split off a separate test that only checks the alignment of various calls, rather than reuse the byval bitcast test. Otherwise LGTM, but I'd wait on someone more familiar with ptx, e.g. ~tra

Dec 29 2022, 6:17 AM · Restricted Project, Restricted Project
pavelkopyl updated the diff for D140581: [NVPTX] Enforce minumum alignment of 4 for byval parametrs in a function prototype.
  • Rebased
  • Added fixes according to review notes
Dec 29 2022, 6:12 AM · Restricted Project, Restricted Project
pavelkopyl added a reviewer for D140754: [NFC][Assignment Tracking Analysis] Add x86 triple to lower-offset-expression.ll: Orlando.
Dec 29 2022, 1:36 AM · Restricted Project, Restricted Project
pavelkopyl requested review of D140754: [NFC][Assignment Tracking Analysis] Add x86 triple to lower-offset-expression.ll.
Dec 29 2022, 1:33 AM · Restricted Project, Restricted Project

Dec 22 2022

pavelkopyl added reviewers for D140581: [NVPTX] Enforce minumum alignment of 4 for byval parametrs in a function prototype: tra, ldrumm.
Dec 22 2022, 12:43 PM · Restricted Project, Restricted Project
pavelkopyl requested review of D140581: [NVPTX] Enforce minumum alignment of 4 for byval parametrs in a function prototype.
Dec 22 2022, 12:39 PM · Restricted Project, Restricted Project

Dec 20 2022

pavelkopyl added a reviewer for D140436: [NVPTX] Add support of ptxas v12.0 to the LIT tests: tra.
Dec 20 2022, 3:12 PM · Restricted Project, Restricted Project
pavelkopyl requested review of D140436: [NVPTX] Add support of ptxas v12.0 to the LIT tests.
Dec 20 2022, 3:11 PM · Restricted Project, Restricted Project
pavelkopyl updated the diff for D140238: [NVPTX] Emit .noreturn directive.
  • Rebased
  • Added run of ptxas to the test
Dec 20 2022, 4:54 AM · Restricted Project, Restricted Project

Dec 16 2022

pavelkopyl updated the diff for D140238: [NVPTX] Emit .noreturn directive.
  • Added fixes according to review comments
Dec 16 2022, 3:46 PM · Restricted Project, Restricted Project
pavelkopyl added inline comments to D140238: [NVPTX] Emit .noreturn directive.
Dec 16 2022, 3:39 PM · Restricted Project, Restricted Project
pavelkopyl added a comment to D140238: [NVPTX] Emit .noreturn directive.

LGTM. Thank you for the patch.

Just curious, does it buy us any measurable improvements?

Dec 16 2022, 3:24 PM · Restricted Project, Restricted Project
pavelkopyl retitled D140238: [NVPTX] Emit .noreturn directive from [NVPTX] Emit .noreturn directive` to [NVPTX] Emit .noreturn directive.
Dec 16 2022, 11:38 AM · Restricted Project, Restricted Project
pavelkopyl updated the diff for D140238: [NVPTX] Emit .noreturn directive.
  • Fix typo in commit msg.
Dec 16 2022, 11:26 AM · Restricted Project, Restricted Project
pavelkopyl requested review of D140238: [NVPTX] Emit .noreturn directive.
Dec 16 2022, 11:21 AM · Restricted Project, Restricted Project

Dec 13 2022

pavelkopyl updated the diff for D138531: [PATCH] [NVPTX] Backend support for variadic functions.
  • Removed unneeded comment from vaarg.ll test
  • Removed unneeded comment from LowerFormalArguments()
Dec 13 2022, 6:39 AM · Restricted Project, Restricted Project
pavelkopyl added inline comments to D138531: [PATCH] [NVPTX] Backend support for variadic functions.
Dec 13 2022, 6:05 AM · Restricted Project, Restricted Project
pavelkopyl updated the diff for D138531: [PATCH] [NVPTX] Backend support for variadic functions.
  • Rebased
  • Added description for getParamSymbol()
Dec 13 2022, 3:48 AM · Restricted Project, Restricted Project

Dec 11 2022

pavelkopyl updated the diff for D138531: [PATCH] [NVPTX] Backend support for variadic functions.

Replace fixed %VAParam name with <function>_vararg.

Dec 11 2022, 11:27 AM · Restricted Project, Restricted Project
pavelkopyl added inline comments to D138531: [PATCH] [NVPTX] Backend support for variadic functions.
Dec 11 2022, 11:23 AM · Restricted Project, Restricted Project

Dec 8 2022

pavelkopyl updated the diff for D138531: [PATCH] [NVPTX] Backend support for variadic functions.

Fix regexp for .align

Dec 8 2022, 2:22 AM · Restricted Project, Restricted Project

Dec 7 2022

pavelkopyl updated the diff for D138531: [PATCH] [NVPTX] Backend support for variadic functions.

Move align hardcode to NVPTXSubtarget where it's available via getMaxRequiredAlignment()

Dec 7 2022, 3:14 PM · Restricted Project, Restricted Project
pavelkopyl added inline comments to D138531: [PATCH] [NVPTX] Backend support for variadic functions.
Dec 7 2022, 3:12 PM · Restricted Project, Restricted Project

Dec 3 2022

pavelkopyl updated the diff for D138531: [PATCH] [NVPTX] Backend support for variadic functions.

Updated vaargs.ll test to make it more clear how vaarg related instructions / intrinsics get lowered.

Dec 3 2022, 4:58 AM · Restricted Project, Restricted Project
pavelkopyl added inline comments to D138531: [PATCH] [NVPTX] Backend support for variadic functions.
Dec 3 2022, 4:55 AM · Restricted Project, Restricted Project
pavelkopyl added a comment to D138531: [PATCH] [NVPTX] Backend support for variadic functions.

Note that aggregates passed by value as variadic arguments are not currently supported.

What happens when a user does try to pass an aggregate as a var arg?

That will trigger llvm_unreachable() at llvm/lib/CodeGen/ValueTypes.cpp:551
But this is common issue - aggregates are not allowed (at least now) in variadic arguments.

Dec 3 2022, 4:50 AM · Restricted Project, Restricted Project

Nov 23 2022

pavelkopyl added inline comments to D138531: [PATCH] [NVPTX] Backend support for variadic functions.
Nov 23 2022, 4:56 PM · Restricted Project, Restricted Project
pavelkopyl updated the diff for D138531: [PATCH] [NVPTX] Backend support for variadic functions.

Fix issue after rebasing.

Nov 23 2022, 4:39 PM · Restricted Project, Restricted Project

Nov 22 2022

pavelkopyl added reviewers for D138531: [PATCH] [NVPTX] Backend support for variadic functions: tra, krisb, kovdan01.
Nov 22 2022, 4:38 PM · Restricted Project, Restricted Project
pavelkopyl requested review of D138531: [PATCH] [NVPTX] Backend support for variadic functions.
Nov 22 2022, 4:26 PM · Restricted Project, Restricted Project