Page MenuHomePhabricator

Encode alignment attribute for `atomicrmw`
Needs ReviewPublic

Authored by gchatelet on Jul 9 2020, 1:29 AM.

Details

Summary

This is a follow up patch to D83136 adding the align attribute to atomicwmw.

Diff Detail

Unit TestsFailed

TimeTest
140 mslinux > LLVM.Transforms/GCOVProfiling::atomic-counter.ll
Script: -- : 'RUN: at line 2'; mkdir -p /mnt/disks/ssd0/agent/llvm-project/build/test/Transforms/GCOVProfiling/Output/atomic-counter.ll.tmp && cd /mnt/disks/ssd0/agent/llvm-project/build/test/Transforms/GCOVProfiling/Output/atomic-counter.ll.tmp
120 mswindows > LLVM.Transforms/GCOVProfiling::atomic-counter.ll
Script: -- : 'RUN: at line 2'; mkdir -p C:\ws\w16c2-1\llvm-project\premerge-checks\build\test\Transforms\GCOVProfiling\Output\atomic-counter.ll.tmp && cd C:\ws\w16c2-1\llvm-project\premerge-checks\build\test\Transforms\GCOVProfiling\Output\atomic-counter.ll.tmp

Event Timeline

gchatelet created this revision.Jul 9 2020, 1:29 AM
jfb added inline comments.Jul 9 2020, 8:23 AM
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
5139

I think you want this instead:

MaybeAlign Align;
if (Error Err = parseAlignmentValue(Record[6], Align))
  return Err;

?

llvm/test/Bitcode/compatibility.ll
756

I think you still need tests *without* alignment?

jyknight added inline comments.Jul 29 2020, 3:38 PM
llvm/docs/LangRef.rst
9538

It's worth explicitly noting here that this default alignment assumption is different than the alignment used for the load/store instructions when align isn't specified.

gchatelet updated this revision to Diff 290818.Wed, Sep 9, 1:19 PM
gchatelet marked 3 inline comments as done.

rebase

llvm/lib/Bitcode/Reader/BitcodeReader.cpp
5139

Absolutely!

gchatelet updated this revision to Diff 290915.Thu, Sep 10, 2:39 AM
  • Use non trivial alignment in tests