Skip to content

Commit

Permalink
Rewrite the llvm-opt-transformer functionality (compiler-explorer#6749)
Browse files Browse the repository at this point in the history 
… to be synchronous and not fail on bad document
Fixes compiler-explorer#6745.

One problem is described in compiler-explorer#6745 (comment) . Also, moved to the library function `yaml.parseAllDocuments` which doesn't throw.  Also, stopped `generateIR` from interfering with opt.yaml writes.
  • Loading branch information
@OfekShilon
OfekShilon authored Aug 4, 2024
1 parent af18032 commit d898152
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 118 deletions.
32 changes: 7 additions & 25 deletions lib/base-compiler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
// POSSIBILITY OF SUCH DAMAGE.

import stream from 'node:stream';
import os from 'os';
import path from 'path';

Expand Down Expand Up @@ -93,7 +92,7 @@ import {InstructionSets} from './instructionsets.js';
import {languages} from './languages.js';
import {LlvmAstParser} from './llvm-ast.js';
import {LlvmIrParser} from './llvm-ir.js';
import * as compilerOptInfo from './llvm-opt-transformer.js';
import {LLVMOptInfo, processRawOptRemarks} from './llvm-opt-transformer.js';
import {logger} from './logger.js';
import {getObjdumperTypeByKey} from './objdumper/index.js';
import {ClientOptionsType, OptionsHandlerLibrary, VersionInfo} from './options-handler.js';
Expand All @@ -104,7 +103,6 @@ import {LlvmPassDumpParser} from './parsers/llvm-pass-dump-parser.js';
import type {PropertyGetter} from './properties.interfaces.js';
import {propsFor} from './properties.js';
import {HeaptrackWrapper} from './runtime-tools/heaptrack-wrapper.js';
import {SentryCapture} from './sentry.js';
import * as StackUsageTransformer from './stack-usage-transformer.js';
import {
clang_style_sysroot_flag,
Expand Down Expand Up @@ -1293,11 +1291,11 @@ export class BaseCompiler implements ICompiler {
produceCfg: boolean,
filters: ParseFiltersAndOutputOptions,
) {
// These options make Clang produce an IR
const newOptions = options
// `-E` causes some mayhem. See #5854
.filter(option => option !== '-fcolor-diagnostics' && option !== '-E')
.concat(unwrap(this.compiler.irArg));
// `-E` /`-fsave-optimization-record` switches caused simultaneus writes into some output files,
// see bugs #5854 / #6745
.filter(option => !['-fcolor-diagnostics', '-E', '-fsave-optimization-record'].includes(option))
.concat(unwrap(this.compiler.irArg)); // produce IR

if (irOptions.noDiscardValueNames && this.compiler.optPipeline?.noDiscardValueNamesArg) {
newOptions.push(...this.compiler.optPipeline.noDiscardValueNamesArg);
Expand Down Expand Up @@ -2927,24 +2925,8 @@ export class BaseCompiler implements ICompiler {
}

async processOptOutput(optPath: string) {
const output: compilerOptInfo.LLVMOptInfo[] = [];

const optStream = stream.pipeline(
fs.createReadStream(optPath, {encoding: 'utf8'}),
new compilerOptInfo.LLVMOptTransformer(),
async err => {
if (err) {
logger.error(`Error handling opt output: ${err}`);
SentryCapture(err, `Error handling opt output: ${await fs.readFile(optPath, 'utf8')}`);
}
},
);

for await (const opt of optStream as AsyncIterable<compilerOptInfo.LLVMOptInfo>) {
if (opt.DebugLoc && opt.DebugLoc.File && opt.DebugLoc.File.includes(this.compileFilename)) {
output.push(opt);
}
}
const optRemarksRaw = await fs.readFile(optPath, 'utf8');
const output: LLVMOptInfo[] = processRawOptRemarks(optRemarksRaw, this.compileFilename);

if (this.compiler.demangler) {
const result = JSON.stringify(output, null, 4);
Expand Down
81 changes: 20 additions & 61 deletions lib/llvm-opt-transformer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,10 @@
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.

import {Transform, TransformCallback, TransformOptions} from 'stream';

import yaml from 'yaml';
import {parseAllDocuments} from 'yaml';

import {logger} from './logger.js';

type Path = string;
type OptType = 'Missed' | 'Passed' | 'Analysis';

type OptInfo = {
Expand All @@ -40,7 +37,7 @@ export type LLVMOptInfo = OptInfo & {
};

type DebugLoc = {
File: Path;
File: string;
Line: number;
Column: number;
};
Expand All @@ -63,65 +60,27 @@ function DisplayOptInfo(optInfo: LLVMOptInfo) {
return displayString;
}

const optTypeMatcher = /---\s(.*)\r?\n/;
const docStart = '---';
const docEnd = '\n...';
const IsDocumentStart = (x: string) => x.startsWith(docStart);
const FindDocumentEnd = (x: string) => {
const index = x.indexOf(docEnd);
return {found: index > -1, endpos: index + docEnd.length};
};
const splitAt = (index, xs) => [xs.slice(0, index), xs.slice(index)];

export class LLVMOptTransformer extends Transform {
_buffer: string;
_prevOpts: Set<string>; // Avoid duplicate display of remarks
constructor(options?: TransformOptions) {
super({...options, objectMode: true});
this._buffer = '';
this._prevOpts = new Set<string>();
}

override _flush(done: TransformCallback) {
this.processBuffer();
done();
}

override _transform(chunk: any, encoding: string, done: TransformCallback) {
// See https://stackoverflow.com/a/40928431/390318 - we have to catch all exceptions here
try {
this._buffer += chunk.toString();
this.processBuffer();
} catch (exception) {
done(exception as Error);
return;
export function processRawOptRemarks(buffer: string, compileFileName: string = ''): LLVMOptInfo[] {
const output: LLVMOptInfo[] = [];
const remarksSet: Set<string> = new Set<string>();
const remarks: any = parseAllDocuments(buffer);
for (const doc of remarks) {
if (doc.errors !== undefined && doc.errors.length > 0) {
logger.warn('YAMLParseError: ' + JSON.stringify(doc.errors[0]));
continue;
}
done();
}

processBuffer() {
while (IsDocumentStart(this._buffer)) {
const {found, endpos} = FindDocumentEnd(this._buffer);
if (found) {
const [head, tail] = splitAt(endpos, this._buffer);
const optTypeMatch = head.match(optTypeMatcher);
const opt = yaml.parse(head, {logLevel: 'error'});
const strOpt = JSON.stringify(opt);
if (!this._prevOpts.has(strOpt)) {
this._prevOpts.add(strOpt);
const opt = doc.toJS();
if (!opt.DebugLoc || !opt.DebugLoc.File || !opt.DebugLoc.File.includes(compileFileName)) continue;

if (optTypeMatch) {
opt.optType = optTypeMatch[1].replace('!', '');
} else {
logger.warn('missing optimization type');
}
opt.displayString = DisplayOptInfo(opt);
this.push(opt as LLVMOptInfo);
}
this._buffer = tail.replace(/^\n/, '');
} else {
break;
}
const strOpt = JSON.stringify(opt);
if (!remarksSet.has(strOpt)) {
remarksSet.add(strOpt);
opt.optType = doc.contents.tag.substring(1); // remove leading '!'
opt.displayString = DisplayOptInfo(opt);
output.push(opt as LLVMOptInfo);
}
}

return output;
}
34 changes: 2 additions & 32 deletions test/llvm-opt-transformer-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,9 @@
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.

import * as stream from 'stream';

import {describe, expect, it} from 'vitest';
import {YAMLParseError} from 'yaml';

import {LLVMOptTransformer} from '../lib/llvm-opt-transformer.js';
import {processRawOptRemarks} from '../lib/llvm-opt-transformer.js';

describe('LLVM opt transformer', () => {
it('should work', async () => {
Expand All @@ -45,15 +42,8 @@ Args:
- String: ' instructions in function'
...
`;
const readString = new stream.PassThrough();
readString.push(doc);
readString.end();
const optStream = readString.pipe(new LLVMOptTransformer());

const output: object[] = [];
for await (const opt of optStream) {
output.push(opt);
}
const output: object[] = processRawOptRemarks(doc);
expect(output).toEqual([
{
Args: [
Expand Down Expand Up @@ -97,24 +87,4 @@ Args:
},
]);
});
it('should error with unparseable yaml data', async () => {
const doc = `--- !Analysis
broken: not a yaml doc
broken: duplicate key makes this invalid
...
`;
const readString = new stream.PassThrough();
readString.push(doc);
readString.end();
await expect(
(async () => {
const optStream = stream.pipeline(readString, new LLVMOptTransformer(), res => {
return res;
});
for await (const _ of optStream) {
// just consume
}
})(),
).rejects.toThrow(YAMLParseError);
});
});

0 comments on commit d898152

Please sign in to comment.